From 62630d6449cc986d2a23f25e7d08c029929c69ef Mon Sep 17 00:00:00 2001 From: Monis Khan Date: Fri, 19 Feb 2021 10:10:30 -0500 Subject: [PATCH] getAggregatedAPIServerScheme: move group version logic internally Signed-off-by: Monis Khan --- internal/concierge/server/server.go | 29 +++++++++++------------- internal/concierge/server/server_test.go | 16 ++++++------- 2 files changed, 21 insertions(+), 24 deletions(-) diff --git a/internal/concierge/server/server.go b/internal/concierge/server/server.go index 3975d853..eab45c97 100644 --- a/internal/concierge/server/server.go +++ b/internal/concierge/server/server.go @@ -174,19 +174,11 @@ func getAggregatedAPIServerConfig( startControllersPostStartHook func(context.Context), apiGroupSuffix string, ) (*apiserver.Config, error) { - loginConciergeAPIGroup, ok := groupsuffix.Replace(loginv1alpha1.GroupName, apiGroupSuffix) - if !ok { - return nil, fmt.Errorf("cannot make api group from %s/%s", loginv1alpha1.GroupName, apiGroupSuffix) - } - - scheme := getAggregatedAPIServerScheme(loginConciergeAPIGroup, apiGroupSuffix) + scheme, groupVersion := getAggregatedAPIServerScheme(apiGroupSuffix) codecs := serializer.NewCodecFactory(scheme) - defaultEtcdPathPrefix := fmt.Sprintf("/registry/%s", loginConciergeAPIGroup) - groupVersion := schema.GroupVersion{ - Group: loginConciergeAPIGroup, - Version: loginv1alpha1.SchemeGroupVersion.Version, - } + // this is unused for now but it is a safe value that we could use in the future + defaultEtcdPathPrefix := fmt.Sprintf("/pinniped-concierge-registry/%s", apiGroupSuffix) recommendedOptions := genericoptions.NewRecommendedOptions( defaultEtcdPathPrefix, @@ -224,18 +216,23 @@ func getAggregatedAPIServerConfig( return apiServerConfig, nil } -func getAggregatedAPIServerScheme(loginConciergeAPIGroup, apiGroupSuffix string) *runtime.Scheme { +func getAggregatedAPIServerScheme(apiGroupSuffix string) (*runtime.Scheme, schema.GroupVersion) { // standard set up of the server side scheme scheme := runtime.NewScheme() // add the options to empty v1 metav1.AddToGroupVersion(scheme, metav1.Unversioned) - // nothing fancy is required if using the standard group - if loginConciergeAPIGroup == loginv1alpha1.GroupName { + // nothing fancy is required if using the standard group suffix + if apiGroupSuffix == "pinniped.dev" { utilruntime.Must(loginv1alpha1.AddToScheme(scheme)) utilruntime.Must(loginapi.AddToScheme(scheme)) - return scheme + return scheme, loginv1alpha1.SchemeGroupVersion + } + + loginConciergeAPIGroup, ok := groupsuffix.Replace(loginv1alpha1.GroupName, apiGroupSuffix) + if !ok { + panic(fmt.Errorf("cannot make api group from %s/%s", loginv1alpha1.GroupName, apiGroupSuffix)) // static input, impossible case } // we need a temporary place to register our types to avoid double registering them @@ -309,5 +306,5 @@ func getAggregatedAPIServerScheme(loginConciergeAPIGroup, apiGroupSuffix string) credentialRequest.Spec.Authenticator.APIGroup = &restoredGroup }) - return scheme + return scheme, schema.GroupVersion{Group: loginConciergeAPIGroup, Version: loginv1alpha1.SchemeGroupVersion.Version} } diff --git a/internal/concierge/server/server_test.go b/internal/concierge/server/server_test.go index 3bc27181..e37afb96 100644 --- a/internal/concierge/server/server_test.go +++ b/internal/concierge/server/server_test.go @@ -20,7 +20,6 @@ import ( loginapi "go.pinniped.dev/generated/latest/apis/concierge/login" loginv1alpha1 "go.pinniped.dev/generated/latest/apis/concierge/login/v1alpha1" - "go.pinniped.dev/internal/groupsuffix" ) const knownGoodUsage = ` @@ -126,9 +125,10 @@ func Test_getAggregatedAPIServerScheme(t *testing.T) { } tests := []struct { - name string - apiGroupSuffix string - want map[schema.GroupVersionKind]reflect.Type + name string + apiGroupSuffix string + want map[schema.GroupVersionKind]reflect.Type + wantGroupVersion schema.GroupVersion }{ { name: "regular api group", @@ -171,6 +171,7 @@ func Test_getAggregatedAPIServerScheme(t *testing.T) { metav1.Unversioned.WithKind("UpdateOptions"): reflect.TypeOf(&metav1.UpdateOptions{}).Elem(), metav1.Unversioned.WithKind("WatchEvent"): reflect.TypeOf(&metav1.WatchEvent{}).Elem(), }, + wantGroupVersion: regularGV, }, { name: "other api group", @@ -213,16 +214,15 @@ func Test_getAggregatedAPIServerScheme(t *testing.T) { metav1.Unversioned.WithKind("UpdateOptions"): reflect.TypeOf(&metav1.UpdateOptions{}).Elem(), metav1.Unversioned.WithKind("WatchEvent"): reflect.TypeOf(&metav1.WatchEvent{}).Elem(), }, + wantGroupVersion: otherGV, }, } for _, tt := range tests { tt := tt t.Run(tt.name, func(t *testing.T) { - loginConciergeAPIGroup, ok := groupsuffix.Replace("login.concierge.pinniped.dev", tt.apiGroupSuffix) - require.True(t, ok) - - scheme := getAggregatedAPIServerScheme(loginConciergeAPIGroup, tt.apiGroupSuffix) + scheme, gv := getAggregatedAPIServerScheme(tt.apiGroupSuffix) require.Equal(t, tt.want, scheme.AllKnownTypes()) + require.Equal(t, tt.wantGroupVersion, gv) // make a credential request like a client would send authenticationConciergeAPIGroup := "authentication.concierge." + tt.apiGroupSuffix