diff --git a/pkg/admission/clusterworkspaceshard/admission.go b/pkg/admission/clusterworkspaceshard/admission.go index 9f318bb3255..282df54b6a9 100644 --- a/pkg/admission/clusterworkspaceshard/admission.go +++ b/pkg/admission/clusterworkspaceshard/admission.go @@ -18,17 +18,16 @@ package clusterworkspaceshard import ( "context" - "errors" "fmt" "io" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/util/validation/field" "k8s.io/apiserver/pkg/admission" "github.com/kcp-dev/kcp/pkg/admission/initializers" tenancyv1alpha1 "github.com/kcp-dev/kcp/pkg/apis/tenancy/v1alpha1" - tenancyhelper "github.com/kcp-dev/kcp/pkg/apis/tenancy/v1alpha1/helper" ) // Validate ClusterWorkspace creation and updates for @@ -53,6 +52,7 @@ type clusterWorkspaceShard struct { *admission.Handler shardBaseURL string + shardExternalURL string externalAddressProvider func() string } @@ -60,11 +60,12 @@ type clusterWorkspaceShard struct { var _ = admission.ValidationInterface(&clusterWorkspaceShard{}) var _ = admission.MutationInterface(&clusterWorkspaceShard{}) var _ = initializers.WantsExternalAddressProvider(&clusterWorkspaceShard{}) +var _ = initializers.WantsShardExternalURL(&clusterWorkspaceShard{}) // Validate ensures that // - baseURL is set // - externalURL is set -func (o *clusterWorkspaceShard) Validate(ctx context.Context, a admission.Attributes, _ admission.ObjectInterfaces) (err error) { +func (o *clusterWorkspaceShard) Validate(_ context.Context, a admission.Attributes, _ admission.ObjectInterfaces) (err error) { if a.GetResource().GroupResource() != tenancyv1alpha1.Resource("clusterworkspaceshards") { return nil } @@ -78,18 +79,24 @@ func (o *clusterWorkspaceShard) Validate(ctx context.Context, a admission.Attrib return fmt.Errorf("failed to convert unstructured to ClusterWorkspace: %w", err) } + var errs field.ErrorList + if cws.Spec.BaseURL == "" { - return admission.NewForbidden(a, errors.New("spec.baseURL must be set")) + errs = append(errs, field.Required(field.NewPath("spec", "baseURL"), "")) } if cws.Spec.ExternalURL == "" { - return admission.NewForbidden(a, errors.New("spec.externalURL must be set")) + errs = append(errs, field.Required(field.NewPath("spec", "externalURL"), "")) + } + + if len(errs) > 0 { + return admission.NewForbidden(a, errs.ToAggregate()) } return nil } -// Admit defaults the baseURL and externalURL to the shards external hostname. -func (o *clusterWorkspaceShard) Admit(ctx context.Context, a admission.Attributes, _ admission.ObjectInterfaces) (err error) { +// Admit sets. +func (o *clusterWorkspaceShard) Admit(_ context.Context, a admission.Attributes, _ admission.ObjectInterfaces) (err error) { if a.GetResource().GroupResource() != tenancyv1alpha1.Resource("clusterworkspaceshards") { return nil } @@ -103,25 +110,27 @@ func (o *clusterWorkspaceShard) Admit(ctx context.Context, a admission.Attribute return fmt.Errorf("failed to convert unstructured to ClusterWorkspaceShard: %w", err) } - if cws.Spec.BaseURL == "" { - defaultExternalAddress := "" - if o.externalAddressProvider != nil { - defaultExternalAddress = o.externalAddressProvider() - } - - if o.shardBaseURL == "" && defaultExternalAddress == "" { - return fmt.Errorf("cannot default baseURL for ClusterWorkspaceShard %q: no default shard base URL and no default external address provided", tenancyhelper.QualifiedObjectName(cws)) - } + externalAddress := "" + if o.externalAddressProvider != nil { + externalAddress = o.externalAddressProvider() + } - if o.shardBaseURL != "" { + if cws.Spec.BaseURL == "" { + switch { + case o.shardBaseURL != "": cws.Spec.BaseURL = o.shardBaseURL - } else { - cws.Spec.BaseURL = "https://" + defaultExternalAddress + case externalAddress != "": + cws.Spec.BaseURL = "https://" + externalAddress } } if cws.Spec.ExternalURL == "" { - cws.Spec.ExternalURL = cws.Spec.BaseURL + switch { + case o.shardExternalURL != "": + cws.Spec.ExternalURL = o.shardExternalURL + case externalAddress != "": + cws.Spec.ExternalURL = "https://" + externalAddress + } } raw, err := runtime.DefaultUnstructuredConverter.ToUnstructured(cws) @@ -137,6 +146,10 @@ func (o *clusterWorkspaceShard) SetShardBaseURL(shardBaseURL string) { o.shardBaseURL = shardBaseURL } +func (o *clusterWorkspaceShard) SetShardExternalURL(shardExternalURL string) { + o.shardExternalURL = shardExternalURL +} + func (o *clusterWorkspaceShard) SetExternalAddressProvider(externalAddressProvider func() string) { o.externalAddressProvider = externalAddressProvider } diff --git a/pkg/admission/clusterworkspaceshard/admission_test.go b/pkg/admission/clusterworkspaceshard/admission_test.go index aba81e75653..59113c238bf 100644 --- a/pkg/admission/clusterworkspaceshard/admission_test.go +++ b/pkg/admission/clusterworkspaceshard/admission_test.go @@ -20,14 +20,12 @@ import ( "context" "testing" + "github.com/google/go-cmp/cmp" "github.com/kcp-dev/logicalcluster" "github.com/stretchr/testify/require" - apiequality "k8s.io/apimachinery/pkg/api/equality" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/util/diff" "k8s.io/apiserver/pkg/admission" "k8s.io/apiserver/pkg/authentication/user" "k8s.io/apiserver/pkg/endpoints/request" @@ -36,13 +34,13 @@ import ( tenancyv1alpha1 "github.com/kcp-dev/kcp/pkg/apis/tenancy/v1alpha1" ) -func createAttr(ws *tenancyv1alpha1.ClusterWorkspaceShard) admission.Attributes { +func createAttr(shard *shardBuilder) admission.Attributes { return admission.NewAttributesRecord( - helpers.ToUnstructuredOrDie(ws), + helpers.ToUnstructuredOrDie(shard.ClusterWorkspaceShard), nil, tenancyv1alpha1.Kind("ClusterWorkspaceShard").WithVersion("v1alpha1"), "", - ws.Name, + shard.Name, tenancyv1alpha1.Resource("clusterworkspaceshards").WithVersion("v1alpha1"), "", admission.Create, @@ -52,13 +50,13 @@ func createAttr(ws *tenancyv1alpha1.ClusterWorkspaceShard) admission.Attributes ) } -func updateAttr(ws, old *tenancyv1alpha1.ClusterWorkspaceShard) admission.Attributes { +func updateAttr(shard, old *shardBuilder) admission.Attributes { return admission.NewAttributesRecord( - helpers.ToUnstructuredOrDie(ws), - helpers.ToUnstructuredOrDie(old), + helpers.ToUnstructuredOrDie(shard.ClusterWorkspaceShard), + helpers.ToUnstructuredOrDie(old.ClusterWorkspaceShard), tenancyv1alpha1.Kind("ClusterWorkspace").WithVersion("v1alpha1"), "", - ws.Name, + shard.Name, tenancyv1alpha1.Resource("clusterworkspaceshards").WithVersion("v1alpha1"), "", admission.Update, @@ -68,353 +66,155 @@ func updateAttr(ws, old *tenancyv1alpha1.ClusterWorkspaceShard) admission.Attrib ) } +type shardBuilder struct { + *tenancyv1alpha1.ClusterWorkspaceShard +} + +func newTestShard() *shardBuilder { + return &shardBuilder{ + ClusterWorkspaceShard: &tenancyv1alpha1.ClusterWorkspaceShard{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + }, + }, + } +} + +func (b *shardBuilder) baseURL(u string) *shardBuilder { + b.Spec.BaseURL = u + return b +} + +func (b *shardBuilder) externalURL(u string) *shardBuilder { + b.Spec.ExternalURL = u + return b +} + +func TestAdmitIgnoresOtherResources(t *testing.T) { + o := &clusterWorkspaceShard{ + Handler: admission.NewHandler(admission.Create, admission.Update), + } + + ctx := request.WithCluster(context.Background(), request.Cluster{Name: logicalcluster.New("root:org")}) + + a := admission.NewAttributesRecord( + &unstructured.Unstructured{}, + nil, + tenancyv1alpha1.Kind("ClusterWorkspace").WithVersion("v1alpha1"), + "", + "test", + tenancyv1alpha1.Resource("clusterworkspaces").WithVersion("v1alpha1"), + "", + admission.Create, + &metav1.CreateOptions{}, + false, + &user.DefaultInfo{}, + ) + + err := o.Admit(ctx, a, nil) + require.NoError(t, err) + require.Equal(t, &unstructured.Unstructured{}, a.GetObject()) +} + +func TestNoOp(t *testing.T) { + shard := newTestShard().baseURL("https://boston2.kcp.dev").externalURL("https://kcp2.dev") + attrs := map[string]admission.Attributes{ + "create": createAttr(shard), + "update": updateAttr(shard, shard), + } + + unstructuredShard := helpers.ToUnstructuredOrDie(shard.ClusterWorkspaceShard) + + for aType, a := range attrs { + t.Run(aType, func(t *testing.T) { + o := &clusterWorkspaceShard{ + Handler: admission.NewHandler(admission.Create, admission.Update), + } + + ctx := request.WithCluster(context.Background(), request.Cluster{Name: logicalcluster.New("root:org")}) + + err := o.Admit(ctx, a, nil) + require.NoError(t, err) + require.Empty(t, cmp.Diff(unstructuredShard, a.GetObject())) + + err = o.Validate(ctx, a, nil) + require.NoError(t, err) + require.Empty(t, cmp.Diff(unstructuredShard, a.GetObject())) + }) + } +} + func TestAdmit(t *testing.T) { tests := []struct { name string - a admission.Attributes emptyExternalAddress bool noExternalAddressProvider bool - expectedObj runtime.Object - wantErr bool + emptyShardExternalURL bool + emptyShardBaseURL bool + expectedObj *shardBuilder }{ { - name: "does nothing on update when baseURL and externalURL are set", - a: updateAttr(&tenancyv1alpha1.ClusterWorkspaceShard{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", - }, - Spec: tenancyv1alpha1.ClusterWorkspaceShardSpec{ - BaseURL: "https://boston2.kcp.dev", - ExternalURL: "https://kcp2.dev", - }, - }, - &tenancyv1alpha1.ClusterWorkspaceShard{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", - }, - Spec: tenancyv1alpha1.ClusterWorkspaceShardSpec{ - BaseURL: "https://boston.kcp.dev", - ExternalURL: "https://kcp.dev", - }, - }), - expectedObj: &tenancyv1alpha1.ClusterWorkspaceShard{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", - }, - Spec: tenancyv1alpha1.ClusterWorkspaceShardSpec{ - BaseURL: "https://boston2.kcp.dev", - ExternalURL: "https://kcp2.dev", - }, - }, - }, - { - name: "does nothing on create when baseURL and externalURL are set", - a: createAttr(&tenancyv1alpha1.ClusterWorkspaceShard{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", - }, - Spec: tenancyv1alpha1.ClusterWorkspaceShardSpec{ - BaseURL: "https://boston2.kcp.dev", - ExternalURL: "https://kcp2.dev", - }, - }), - expectedObj: &tenancyv1alpha1.ClusterWorkspaceShard{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", - }, - Spec: tenancyv1alpha1.ClusterWorkspaceShardSpec{ - BaseURL: "https://boston2.kcp.dev", - ExternalURL: "https://kcp2.dev", - }, - }, - }, - { - name: "default externalURL on update when baseURL is set", - a: updateAttr(&tenancyv1alpha1.ClusterWorkspaceShard{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", - }, - Spec: tenancyv1alpha1.ClusterWorkspaceShardSpec{ - BaseURL: "https://boston2.kcp.dev", - }, - }, - &tenancyv1alpha1.ClusterWorkspaceShard{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", - }, - Spec: tenancyv1alpha1.ClusterWorkspaceShardSpec{ - BaseURL: "https://boston2.kcp.dev", - ExternalURL: "https://kcp.dev", - }, - }), - expectedObj: &tenancyv1alpha1.ClusterWorkspaceShard{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", - }, - Spec: tenancyv1alpha1.ClusterWorkspaceShardSpec{ - BaseURL: "https://boston2.kcp.dev", - ExternalURL: "https://boston2.kcp.dev", - }, - }, - }, - { - name: "default externalURL on create when baseURL is set", - a: createAttr(&tenancyv1alpha1.ClusterWorkspaceShard{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", - }, - Spec: tenancyv1alpha1.ClusterWorkspaceShardSpec{ - BaseURL: "https://boston2.kcp.dev", - }, - }), - expectedObj: &tenancyv1alpha1.ClusterWorkspaceShard{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", - }, - Spec: tenancyv1alpha1.ClusterWorkspaceShardSpec{ - BaseURL: "https://boston2.kcp.dev", - ExternalURL: "https://boston2.kcp.dev", - }, - }, - }, - { - name: "default baseURL on update", - a: updateAttr(&tenancyv1alpha1.ClusterWorkspaceShard{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", - }, - Spec: tenancyv1alpha1.ClusterWorkspaceShardSpec{ - ExternalURL: "https://kcp.dev", - }, - }, - &tenancyv1alpha1.ClusterWorkspaceShard{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", - }, - Spec: tenancyv1alpha1.ClusterWorkspaceShardSpec{ - BaseURL: "https://boston.kcp.dev", - ExternalURL: "https://kcp.dev", - }, - }), - expectedObj: &tenancyv1alpha1.ClusterWorkspaceShard{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", - }, - Spec: tenancyv1alpha1.ClusterWorkspaceShardSpec{ - BaseURL: "https://external.kcp.dev", - ExternalURL: "https://kcp.dev", - }, - }, - }, - { - name: "default baseURL on create", - a: createAttr(&tenancyv1alpha1.ClusterWorkspaceShard{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", - }, - Spec: tenancyv1alpha1.ClusterWorkspaceShardSpec{ - ExternalURL: "https://kcp.dev", - }, - }), - expectedObj: &tenancyv1alpha1.ClusterWorkspaceShard{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", - }, - Spec: tenancyv1alpha1.ClusterWorkspaceShardSpec{ - BaseURL: "https://external.kcp.dev", - ExternalURL: "https://kcp.dev", - }, - }, - }, - { - name: "default externalURL and baseURL on update", - a: updateAttr(&tenancyv1alpha1.ClusterWorkspaceShard{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", - }, - Spec: tenancyv1alpha1.ClusterWorkspaceShardSpec{}, - }, - &tenancyv1alpha1.ClusterWorkspaceShard{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", - }, - Spec: tenancyv1alpha1.ClusterWorkspaceShardSpec{ - BaseURL: "https://boston.kcp.dev", - ExternalURL: "https://kcp.dev", - }, - }), - expectedObj: &tenancyv1alpha1.ClusterWorkspaceShard{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", - }, - Spec: tenancyv1alpha1.ClusterWorkspaceShardSpec{ - BaseURL: "https://external.kcp.dev", - ExternalURL: "https://external.kcp.dev", - }, - }, - }, - { - name: "default baseURL on create", - a: createAttr(&tenancyv1alpha1.ClusterWorkspaceShard{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", - }, - Spec: tenancyv1alpha1.ClusterWorkspaceShardSpec{}, - }), - expectedObj: &tenancyv1alpha1.ClusterWorkspaceShard{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", - }, - Spec: tenancyv1alpha1.ClusterWorkspaceShardSpec{ - BaseURL: "https://external.kcp.dev", - ExternalURL: "https://external.kcp.dev", - }, - }, - }, - { - name: "ignores different resources", - a: admission.NewAttributesRecord( - &unstructured.Unstructured{Object: map[string]interface{}{ - "apiVersion": tenancyv1alpha1.SchemeGroupVersion.String(), - "kind": "ClusterWorkspace", - "metadata": map[string]interface{}{ - "name": "test", - "creationTimestamp": nil, - }, - "spec": map[string]interface{}{ - "type": map[string]interface{}{ - "name": "", - "path": "", - }, - }, - "status": map[string]interface{}{ - "location": map[string]interface{}{}, - }, - }}, - nil, - tenancyv1alpha1.Kind("ClusterWorkspace").WithVersion("v1alpha1"), - "", - "test", - tenancyv1alpha1.Resource("clusterworkspaces").WithVersion("v1alpha1"), - "", - admission.Create, - &metav1.CreateOptions{}, - false, - &user.DefaultInfo{}, - ), - expectedObj: &tenancyv1alpha1.ClusterWorkspace{ - TypeMeta: metav1.TypeMeta{ - APIVersion: tenancyv1alpha1.SchemeGroupVersion.String(), - Kind: "ClusterWorkspace", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "test", - }, - }, - }, - { - name: "default externalURL on create when baseURL is set", - a: createAttr(&tenancyv1alpha1.ClusterWorkspaceShard{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", - }, - Spec: tenancyv1alpha1.ClusterWorkspaceShardSpec{ - BaseURL: "https://boston2.kcp.dev", - }, - }), - noExternalAddressProvider: true, - expectedObj: &tenancyv1alpha1.ClusterWorkspaceShard{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", - }, - Spec: tenancyv1alpha1.ClusterWorkspaceShardSpec{ - BaseURL: "https://boston2.kcp.dev", - ExternalURL: "https://boston2.kcp.dev", - }, - }, - }, - { - name: "fails on create when baseURL is not set and external address provider is nil", - a: createAttr(&tenancyv1alpha1.ClusterWorkspaceShard{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", - }, - Spec: tenancyv1alpha1.ClusterWorkspaceShardSpec{}, - }), - noExternalAddressProvider: true, - wantErr: true, + name: "default baseURL to shardBaseURL when both shardBaseURL and externalAddress are set", + expectedObj: newTestShard().baseURL("https://shard.base").externalURL("https://shard.external"), }, { - name: "fails on update when baseURL is not set and external address provider is nil", - a: updateAttr(&tenancyv1alpha1.ClusterWorkspaceShard{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", - }, - Spec: tenancyv1alpha1.ClusterWorkspaceShardSpec{}, - }, - &tenancyv1alpha1.ClusterWorkspaceShard{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", - }, - Spec: tenancyv1alpha1.ClusterWorkspaceShardSpec{}, - }), - noExternalAddressProvider: true, - wantErr: true, + name: "default baseURL to externalAddress when only externalAddress is set", + emptyShardBaseURL: true, + expectedObj: newTestShard().baseURL("https://external.kcp.dev").externalURL("https://shard.external"), }, { - name: "fails on create when baseURL is not set and external address provider returns empty string", - a: createAttr(&tenancyv1alpha1.ClusterWorkspaceShard{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", - }, - Spec: tenancyv1alpha1.ClusterWorkspaceShardSpec{}, - }), - emptyExternalAddress: true, - wantErr: true, + name: "default externalURL to shardExternalURL when both shardExternalURL and externalAddress are set", + expectedObj: newTestShard().baseURL("https://shard.base").externalURL("https://shard.external"), }, { - name: "fails on update when baseURL is not set and external address provider returns empty string", - a: updateAttr(&tenancyv1alpha1.ClusterWorkspaceShard{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", - }, - Spec: tenancyv1alpha1.ClusterWorkspaceShardSpec{}, - }, - &tenancyv1alpha1.ClusterWorkspaceShard{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", - }, - Spec: tenancyv1alpha1.ClusterWorkspaceShardSpec{}, - }), - emptyExternalAddress: true, - wantErr: true, + name: "default externalURL to externalAddress when only externalAddress is set", + emptyShardExternalURL: true, + expectedObj: newTestShard().baseURL("https://shard.base").externalURL("https://external.kcp.dev"), }, } for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - o := &clusterWorkspaceShard{ - Handler: admission.NewHandler(admission.Create, admission.Update), - externalAddressProvider: func() string { return "external.kcp.dev" }, - } - if tt.noExternalAddressProvider { - o.externalAddressProvider = nil - } else if tt.emptyExternalAddress { - o.externalAddressProvider = func() string { - return "" + shard := newTestShard() + attrs := map[string]admission.Attributes{ + "create": createAttr(shard), + "update": updateAttr(shard, shard), + } + + for aType, a := range attrs { + t.Run(tt.name+" - "+aType, func(t *testing.T) { + o := &clusterWorkspaceShard{ + Handler: admission.NewHandler(admission.Create, admission.Update), + externalAddressProvider: func() string { return "external.kcp.dev" }, + shardExternalURL: "https://shard.external", + shardBaseURL: "https://shard.base", } - } - ctx := request.WithCluster(context.Background(), request.Cluster{Name: logicalcluster.New("root:org")}) - if err := o.Admit(ctx, tt.a, nil); (err != nil) != tt.wantErr { - t.Fatalf("Validate() error = %v, wantErr %v", err, tt.wantErr) - } else if err == nil { - got, ok := tt.a.GetObject().(*unstructured.Unstructured) - require.True(t, ok, "expected unstructured, got %T", tt.a.GetObject()) - expected := helpers.ToUnstructuredOrDie(tt.expectedObj) - if !apiequality.Semantic.DeepEqual(expected, got) { - t.Fatalf("unexpected result (A expected, B got): %s", diff.ObjectDiff(expected, got)) + + if tt.noExternalAddressProvider { + o.externalAddressProvider = nil + } else if tt.emptyExternalAddress { + o.externalAddressProvider = func() string { + return "" + } } - } - }) + + if tt.emptyShardExternalURL { + o.shardExternalURL = "" + } + + if tt.emptyShardBaseURL { + o.shardBaseURL = "" + } + + ctx := request.WithCluster(context.Background(), request.Cluster{Name: logicalcluster.New("root:org")}) + err := o.Admit(ctx, a, nil) + require.NoError(t, err) + + got, ok := a.GetObject().(*unstructured.Unstructured) + require.True(t, ok, "expected unstructured, got %T", a.GetObject()) + + expected := helpers.ToUnstructuredOrDie(tt.expectedObj.ClusterWorkspaceShard) + require.Empty(t, cmp.Diff(expected, got)) + }) + } } } @@ -426,123 +226,41 @@ func TestValidate(t *testing.T) { }{ { name: "accept non-empty baseURL and externalURL on update", - a: updateAttr(&tenancyv1alpha1.ClusterWorkspaceShard{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", - }, - Spec: tenancyv1alpha1.ClusterWorkspaceShardSpec{ - BaseURL: "https://kcp", - ExternalURL: "https://kcp", - }, - }, - &tenancyv1alpha1.ClusterWorkspaceShard{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", - }, - Spec: tenancyv1alpha1.ClusterWorkspaceShardSpec{ - BaseURL: "https://kcp", - ExternalURL: "https://kcp", - }, - }), + a: updateAttr( + newTestShard().baseURL("https://updatedBase").externalURL("https://updatedExternal"), + newTestShard().baseURL("https://base").externalURL("https://external"), + ), }, { name: "accept non-empty baseURL and externalURL on create", - a: createAttr(&tenancyv1alpha1.ClusterWorkspaceShard{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", - }, - Spec: tenancyv1alpha1.ClusterWorkspaceShardSpec{ - BaseURL: "https://kcp", - ExternalURL: "https://kcp", - }, - }), + a: createAttr(newTestShard().baseURL("https://base").externalURL("https://external")), }, { name: "reject empty baseURL on update", - a: updateAttr(&tenancyv1alpha1.ClusterWorkspaceShard{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", - }, - Spec: tenancyv1alpha1.ClusterWorkspaceShardSpec{ - ExternalURL: "https://kcp", - }, - }, - &tenancyv1alpha1.ClusterWorkspaceShard{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", - }, - Spec: tenancyv1alpha1.ClusterWorkspaceShardSpec{ - BaseURL: "https://kcp", - ExternalURL: "https://kcp", - }, - }), + a: updateAttr( + newTestShard().baseURL("").externalURL("https://updatedExternal"), + newTestShard().baseURL("https://base").externalURL("https://external"), + ), wantErr: true, }, { - name: "reject empty baseURL on create", - a: createAttr(&tenancyv1alpha1.ClusterWorkspaceShard{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", - }, - Spec: tenancyv1alpha1.ClusterWorkspaceShardSpec{ - ExternalURL: "https://kcp", - }, - }), + name: "reject empty baseURL on create", + a: createAttr(newTestShard().externalURL("https://external")), wantErr: true, }, { name: "reject empty externalURL on update", - a: updateAttr(&tenancyv1alpha1.ClusterWorkspaceShard{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", - }, - Spec: tenancyv1alpha1.ClusterWorkspaceShardSpec{ - BaseURL: "https://kcp", - }, - }, - &tenancyv1alpha1.ClusterWorkspaceShard{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", - }, - Spec: tenancyv1alpha1.ClusterWorkspaceShardSpec{ - BaseURL: "https://kcp", - ExternalURL: "https://kcp", - }, - }), + a: updateAttr( + newTestShard().baseURL("https://base").externalURL(""), + newTestShard().baseURL("https://base").externalURL("https://external"), + ), wantErr: true, }, { - name: "reject empty externalURL on create", - a: createAttr(&tenancyv1alpha1.ClusterWorkspaceShard{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", - }, - Spec: tenancyv1alpha1.ClusterWorkspaceShardSpec{ - BaseURL: "https://kcp", - }, - }), + name: "reject empty externalURL on create", + a: createAttr(newTestShard().baseURL("https://base").externalURL("")), wantErr: true, }, - { - name: "ignores different resources", - a: admission.NewAttributesRecord( - &tenancyv1alpha1.ClusterWorkspaceShard{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", - }, - }, - nil, - tenancyv1alpha1.Kind("ClusterWorkspace").WithVersion("v1alpha1"), - "", - "test", - tenancyv1alpha1.Resource("clusterworkspaces").WithVersion("v1alpha1"), - "", - admission.Create, - &metav1.CreateOptions{}, - false, - &user.DefaultInfo{}, - ), - }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -556,3 +274,180 @@ func TestValidate(t *testing.T) { }) } } + +func TestRegister(t *testing.T) { + type args struct { + plugins *admission.Plugins + } + tests := []struct { + name string + args args + }{ + // TODO: Add test cases. + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + Register(tt.args.plugins) + }) + } +} + +func Test_clusterWorkspaceShard_Admit(t *testing.T) { + type fields struct { + Handler *admission.Handler + shardBaseURL string + shardExternalURL string + externalAddressProvider func() string + } + type args struct { + in0 context.Context + a admission.Attributes + in2 admission.ObjectInterfaces + } + tests := []struct { + name string + fields fields + args args + wantErr bool + }{ + // TODO: Add test cases. + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + o := &clusterWorkspaceShard{ + Handler: tt.fields.Handler, + shardBaseURL: tt.fields.shardBaseURL, + shardExternalURL: tt.fields.shardExternalURL, + externalAddressProvider: tt.fields.externalAddressProvider, + } + if err := o.Admit(tt.args.in0, tt.args.a, tt.args.in2); (err != nil) != tt.wantErr { + t.Errorf("Admit() error = %v, wantErr %v", err, tt.wantErr) + } + }) + } +} + +func Test_clusterWorkspaceShard_SetExternalAddressProvider(t *testing.T) { + type fields struct { + Handler *admission.Handler + shardBaseURL string + shardExternalURL string + externalAddressProvider func() string + } + type args struct { + externalAddressProvider func() string + } + tests := []struct { + name string + fields fields + args args + }{ + // TODO: Add test cases. + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + o := &clusterWorkspaceShard{ + Handler: tt.fields.Handler, + shardBaseURL: tt.fields.shardBaseURL, + shardExternalURL: tt.fields.shardExternalURL, + externalAddressProvider: tt.fields.externalAddressProvider, + } + o.SetExternalAddressProvider(tt.args.externalAddressProvider) + }) + } +} + +func Test_clusterWorkspaceShard_SetShardBaseURL(t *testing.T) { + type fields struct { + Handler *admission.Handler + shardBaseURL string + shardExternalURL string + externalAddressProvider func() string + } + type args struct { + shardBaseURL string + } + tests := []struct { + name string + fields fields + args args + }{ + // TODO: Add test cases. + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + o := &clusterWorkspaceShard{ + Handler: tt.fields.Handler, + shardBaseURL: tt.fields.shardBaseURL, + shardExternalURL: tt.fields.shardExternalURL, + externalAddressProvider: tt.fields.externalAddressProvider, + } + o.SetShardBaseURL(tt.args.shardBaseURL) + }) + } +} + +func Test_clusterWorkspaceShard_SetShardExternalURL(t *testing.T) { + type fields struct { + Handler *admission.Handler + shardBaseURL string + shardExternalURL string + externalAddressProvider func() string + } + type args struct { + shardExternalURL string + } + tests := []struct { + name string + fields fields + args args + }{ + // TODO: Add test cases. + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + o := &clusterWorkspaceShard{ + Handler: tt.fields.Handler, + shardBaseURL: tt.fields.shardBaseURL, + shardExternalURL: tt.fields.shardExternalURL, + externalAddressProvider: tt.fields.externalAddressProvider, + } + o.SetShardExternalURL(tt.args.shardExternalURL) + }) + } +} + +func Test_clusterWorkspaceShard_Validate(t *testing.T) { + type fields struct { + Handler *admission.Handler + shardBaseURL string + shardExternalURL string + externalAddressProvider func() string + } + type args struct { + in0 context.Context + a admission.Attributes + in2 admission.ObjectInterfaces + } + tests := []struct { + name string + fields fields + args args + wantErr bool + }{ + // TODO: Add test cases. + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + o := &clusterWorkspaceShard{ + Handler: tt.fields.Handler, + shardBaseURL: tt.fields.shardBaseURL, + shardExternalURL: tt.fields.shardExternalURL, + externalAddressProvider: tt.fields.externalAddressProvider, + } + if err := o.Validate(tt.args.in0, tt.args.a, tt.args.in2); (err != nil) != tt.wantErr { + t.Errorf("Validate() error = %v, wantErr %v", err, tt.wantErr) + } + }) + } +} diff --git a/pkg/admission/initializers/initializer.go b/pkg/admission/initializers/initializer.go index d1e72168e73..a7d68bb4fc6 100644 --- a/pkg/admission/initializers/initializer.go +++ b/pkg/admission/initializers/initializer.go @@ -121,3 +121,21 @@ func (i *shardBaseURLInitializer) Initialize(plugin admission.Interface) { wants.SetShardBaseURL(i.shardBaseURL) } } + +// NewShardExternalURLInitializer returns an admission plugin initializer that injects +// the default shard external URL provider into the admission plugin. +func NewShardExternalURLInitializer(shardExternalURL string) *shardExternalURLInitializer { + return &shardExternalURLInitializer{ + shardExternalURL: shardExternalURL, + } +} + +type shardExternalURLInitializer struct { + shardExternalURL string +} + +func (i *shardExternalURLInitializer) Initialize(plugin admission.Interface) { + if wants, ok := plugin.(WantsShardExternalURL); ok { + wants.SetShardExternalURL(i.shardExternalURL) + } +} diff --git a/pkg/admission/initializers/interfaces.go b/pkg/admission/initializers/interfaces.go index 169fa6d51f0..22113f850c0 100644 --- a/pkg/admission/initializers/interfaces.go +++ b/pkg/admission/initializers/interfaces.go @@ -52,3 +52,9 @@ type WantsExternalAddressProvider interface { type WantsShardBaseURL interface { SetShardBaseURL(shardBaseURL string) } + +// WantsShardExternalURL interface should be implemented by admission plugins +// that want to have the shard external url injected. +type WantsShardExternalURL interface { + SetShardExternalURL(shardExternalURL string) +} diff --git a/pkg/server/options/flags.go b/pkg/server/options/flags.go index d1a89e5280b..a95a1e46269 100644 --- a/pkg/server/options/flags.go +++ b/pkg/server/options/flags.go @@ -130,7 +130,8 @@ var ( "discovery-poll-interval", // Polling interval for dynamic discovery informers. "profiler-address", // [Address]:port to bind the profiler to "root-directory", // Root directory. - "shard-base-url", // Base URL to the this kcp shard. Defaults to external address. + "shard-base-url", // Base URL to this kcp shard. Defaults to external address. + "shard-external-url", // URL used by outside clients to talk to this kcp shard. Defaults to external address. "shard-name", // A name of this kcp shard. "shard-kubeconfig-file", // Kubeconfig holding admin(!) credentials to peer kcp shards. "experimental-bind-free-port", // Bind to a free port. --secure-bind-port must be 0. Use the admin.kubeconfig to extract the chosen port. diff --git a/pkg/server/options/options.go b/pkg/server/options/options.go index f00849b58f7..fd2f04031ee 100644 --- a/pkg/server/options/options.go +++ b/pkg/server/options/options.go @@ -55,6 +55,7 @@ type ExtraOptions struct { ProfilerAddress string ShardKubeconfigFile string ShardBaseURL string + ShardExternalURL string ShardName string DiscoveryPollInterval time.Duration ExperimentalBindFreePort bool @@ -92,6 +93,7 @@ func NewOptions(rootDir string) *Options { ProfilerAddress: "", ShardKubeconfigFile: "", ShardBaseURL: "", + ShardExternalURL: "", ShardName: "root", DiscoveryPollInterval: 60 * time.Second, ExperimentalBindFreePort: false, @@ -108,7 +110,7 @@ func NewOptions(rootDir string) *Options { WithRequestHeader(). WithServiceAccounts(). WithTokenFile() - //WithWebHook() + // WithWebHook() o.GenericControlPlane.Authentication.ServiceAccounts.Issuers = []string{"https://kcp.default.svc"} o.GenericControlPlane.Etcd.StorageConfig.Transport.ServerList = []string{"embedded"} @@ -152,7 +154,8 @@ func (o *Options) rawFlags() cliflag.NamedFlagSets { fs := fss.FlagSet("KCP") fs.StringVar(&o.Extra.ProfilerAddress, "profiler-address", o.Extra.ProfilerAddress, "[Address]:port to bind the profiler to") fs.StringVar(&o.Extra.ShardKubeconfigFile, "shard-kubeconfig-file", o.Extra.ShardKubeconfigFile, "Kubeconfig holding admin(!) credentials to peer kcp shards.") - fs.StringVar(&o.Extra.ShardBaseURL, "shard-base-url", o.Extra.ShardBaseURL, "Base URL to the this kcp shard. Defaults to external address.") + fs.StringVar(&o.Extra.ShardBaseURL, "shard-base-url", o.Extra.ShardBaseURL, "Base URL to this kcp shard. Defaults to external address.") + fs.StringVar(&o.Extra.ShardExternalURL, "shard-external-url", o.Extra.ShardExternalURL, "URL used by outside clients to talk to this kcp shard. Defaults to external address.") fs.StringVar(&o.Extra.ShardName, "shard-name", o.Extra.ShardName, "A name of this kcp shard. Defaults to the \"root\" name.") fs.StringVar(&o.Extra.RootDirectory, "root-directory", o.Extra.RootDirectory, "Root directory.") fs.DurationVar(&o.Extra.DiscoveryPollInterval, "discovery-poll-interval", o.Extra.DiscoveryPollInterval, "Polling interval for dynamic discovery informers.") diff --git a/pkg/server/server.go b/pkg/server/server.go index b7e32544bc5..0294b2f9f77 100644 --- a/pkg/server/server.go +++ b/pkg/server/server.go @@ -274,6 +274,7 @@ func (s *Server) Run(ctx context.Context) error { kcpadmissioninitializers.NewKubeClusterClientInitializer(kubeClusterClient), kcpadmissioninitializers.NewKcpClusterClientInitializer(kcpClusterClient), kcpadmissioninitializers.NewShardBaseURLInitializer(s.options.Extra.ShardBaseURL), + kcpadmissioninitializers.NewShardExternalURLInitializer(s.options.Extra.ShardExternalURL), // The external address is provided as a function, as its value may be updated // with the default secure port, when the config is later completed. kcpadmissioninitializers.NewExternalAddressInitializer(func() string { return genericConfig.ExternalAddress }),