Skip to content

Commit a44c196

Browse files
kcp-ci-botkcp-ci-bot
authored
Merge pull request #3411 from mjudeikis/mjudeikis/disable.VW.url
Disable APIExport VirtualWorkspace URL population
2 parents 204fd33 + fd0b98b commit a44c196

File tree

17 files changed

+432
-271
lines changed

17 files changed

+432
-271
lines changed

‎Makefile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -348,7 +348,7 @@ test-e2e-sharded-minimal: build-all
348348

349349
# This is just easy target to run 2 shard test server locally until manually killed.
350350
# You can targer test to it by running:
351-
# go test ./test/e2e/apibinding/... --kcp-kubeconfig=${WORK_DIR:-.}/.kcp/admin.kubeconfig --shard-kubeconfigs=root=${WORK_DIR:-.}/.kcp-0/admin.kubeconfig -run=^TestAPIBindingEndpointSlicesSharded$
351+
# go test ./test/e2e/apibinding/... --kcp-kubeconfig=$(pwd)/.kcp/admin.kubeconfig --shard-kubeconfigs=root=$(pwd)/.kcp-0/admin.kubeconfig -run=^TestAPIBinding$
352352
test-run-sharded-server: WORK_DIR ?= $(PWD)
353353
test-run-sharded-server: LOG_DIR ?= $(WORK_DIR)/.kcp
354354
test-run-sharded-server:

‎pkg/features/kcp_features.go

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,12 @@ const (
4747
// alpha: v0.1
4848
// Enables cache apis and controllers.
4949
CacheAPIs featuregate.Feature = "CacheAPIs"
50+
51+
// owner: @mjudeikis
52+
// alpha: v0.1
53+
// Enables VirtualWorkspace urls on APIExport. This enables to use Deprecated APIExport VirtualWorkspace urls.
54+
// This is a temporary feature to ease the migration to the new VirtualWorkspace urls.
55+
EnableDeprecatedAPIExportVirtualWorkspacesUrls featuregate.Feature = "EnableDeprecatedAPIExportVirtualWorkspacesUrls"
5056
)
5157

5258
// DefaultFeatureGate exposes the upstream feature gate, but with our gate setting applied.
@@ -120,7 +126,9 @@ var defaultVersionedGenericControlPlaneFeatureGates = map[featuregate.Feature]fe
120126
CacheAPIs: {
121127
{Version: version.MustParse("1.32"), Default: false, PreRelease: featuregate.Alpha},
122128
},
123-
129+
EnableDeprecatedAPIExportVirtualWorkspacesUrls: {
130+
{Version: version.MustParse("1.32"), Default: false, PreRelease: featuregate.Alpha},
131+
},
124132
// inherited features from generic apiserver, relisted here to get a conflict if it is changed
125133
// unintentionally on either side:
126134
genericfeatures.APIResponseCompression: {

‎pkg/reconciler/apis/apiexport/apiexport_controller.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,10 +41,12 @@ import (
4141
"github.com/kcp-dev/kcp/pkg/logging"
4242
"github.com/kcp-dev/kcp/pkg/reconciler/committer"
4343
"github.com/kcp-dev/kcp/pkg/reconciler/events"
44+
apisv1alpha1 "github.com/kcp-dev/kcp/sdk/apis/apis/v1alpha1"
4445
apisv1alpha2 "github.com/kcp-dev/kcp/sdk/apis/apis/v1alpha2"
4546
corev1alpha1 "github.com/kcp-dev/kcp/sdk/apis/core/v1alpha1"
4647
kcpclientset "github.com/kcp-dev/kcp/sdk/client/clientset/versioned/cluster"
4748
apisv1alpha2client "github.com/kcp-dev/kcp/sdk/client/clientset/versioned/typed/apis/v1alpha2"
49+
apisv1alpha1informers "github.com/kcp-dev/kcp/sdk/client/informers/externalversions/apis/v1alpha1"
4850
apisv1alpha2informers "github.com/kcp-dev/kcp/sdk/client/informers/externalversions/apis/v1alpha2"
4951
corev1alpha1informers "github.com/kcp-dev/kcp/sdk/client/informers/externalversions/core/v1alpha1"
5052
)
@@ -59,6 +61,7 @@ const (
5961
func NewController(
6062
kcpClusterClient kcpclientset.ClusterInterface,
6163
apiExportInformer apisv1alpha2informers.APIExportClusterInformer,
64+
apiExportEndpointSliceInformer apisv1alpha1informers.APIExportEndpointSliceClusterInformer,
6265
globalShardInformer corev1alpha1informers.ShardClusterInformer,
6366
kubeClusterClient kcpkubernetesclientset.ClusterInterface,
6467
namespaceInformer kcpcorev1informers.NamespaceClusterInformer,
@@ -89,6 +92,13 @@ func NewController(
8992
getAPIExport: func(clusterName logicalcluster.Name, name string) (*apisv1alpha2.APIExport, error) {
9093
return apiExportInformer.Lister().Cluster(clusterName).Get(name)
9194
},
95+
getAPIExportEndpointSlice: func(clusterName logicalcluster.Name, name string) (*apisv1alpha1.APIExportEndpointSlice, error) {
96+
return apiExportEndpointSliceInformer.Lister().Cluster(clusterName).Get(name)
97+
},
98+
createAPIExportEndpointSlice: func(ctx context.Context, clusterName logicalcluster.Path, apiExportEndpointSlice *apisv1alpha1.APIExportEndpointSlice) error {
99+
_, err := kcpClusterClient.Cluster(clusterName).ApisV1alpha1().APIExportEndpointSlices().Create(ctx, apiExportEndpointSlice, metav1.CreateOptions{})
100+
return err
101+
},
92102

93103
getNamespace: func(clusterName logicalcluster.Name, name string) (*corev1.Namespace, error) {
94104
return namespaceInformer.Lister().Cluster(clusterName).Get(name)
@@ -183,6 +193,9 @@ type controller struct {
183193
listAPIExportsForSecret func(secret *corev1.Secret) ([]*apisv1alpha2.APIExport, error)
184194
getAPIExport func(clusterName logicalcluster.Name, name string) (*apisv1alpha2.APIExport, error)
185195

196+
getAPIExportEndpointSlice func(clusterName logicalcluster.Name, name string) (*apisv1alpha1.APIExportEndpointSlice, error)
197+
createAPIExportEndpointSlice func(ctx context.Context, clusterName logicalcluster.Path, apiExportEndpointSlice *apisv1alpha1.APIExportEndpointSlice) error
198+
186199
getNamespace func(clusterName logicalcluster.Name, name string) (*corev1.Namespace, error)
187200
createNamespace func(ctx context.Context, clusterName logicalcluster.Path, ns *corev1.Namespace) error
188201

‎pkg/reconciler/apis/apiexport/apiexport_controller_test.go

Lines changed: 79 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import (
3131

3232
"github.com/kcp-dev/logicalcluster/v3"
3333

34+
kcpfeatures "github.com/kcp-dev/kcp/pkg/features"
3435
apisv1alpha1 "github.com/kcp-dev/kcp/sdk/apis/apis/v1alpha1"
3536
apisv1alpha2 "github.com/kcp-dev/kcp/sdk/apis/apis/v1alpha2"
3637
corev1alpha1 "github.com/kcp-dev/kcp/sdk/apis/core/v1alpha1"
@@ -39,6 +40,9 @@ import (
3940
)
4041

4142
func TestReconcile(t *testing.T) {
43+
// Save original feature gate value
44+
originalFeatureGate := kcpfeatures.DefaultFeatureGate.Enabled(kcpfeatures.EnableDeprecatedAPIExportVirtualWorkspacesUrls)
45+
4246
tests := map[string]struct {
4347
secretRefSet bool
4448
secretExists bool
@@ -49,19 +53,21 @@ func TestReconcile(t *testing.T) {
4953
apiExportHasSomeOtherHash bool
5054
hasPreexistingVerifyFailure bool
5155
listShardsError error
56+
apiExportEndpointSliceNotFound bool
57+
skipEndpointSliceAnnotation bool
5258

5359
apiBindings []interface{}
5460

55-
wantGenerationFailed bool
56-
wantError bool
57-
wantCreateSecretCalled bool
58-
wantUnsetIdentity bool
59-
wantDefaultSecretRef bool
60-
wantStatusHashSet bool
61-
wantVerifyFailure bool
62-
wantIdentityValid bool
63-
wantVirtualWorkspaceURLsError bool
64-
wantVirtualWorkspaceURLsReady bool
61+
wantGenerationFailed bool
62+
wantError bool
63+
wantCreateSecretCalled bool
64+
wantUnsetIdentity bool
65+
wantDefaultSecretRef bool
66+
wantStatusHashSet bool
67+
wantVerifyFailure bool
68+
wantIdentityValid bool
69+
wantCreateAPIExportEndpointSlice bool
70+
wantVirtualWorkspaceURLs bool
6571
}{
6672
"create secret when ref is nil and secret doesn't exist": {
6773
secretExists: false,
@@ -122,26 +128,55 @@ func TestReconcile(t *testing.T) {
122128
apiBindings: []interface{}{
123129
"something",
124130
},
125-
listShardsError: errors.New("foo"),
126-
wantVirtualWorkspaceURLsError: true,
131+
listShardsError: errors.New("foo"),
127132
},
128-
"virtualWorkspaceURLs set when APIBindings present": {
133+
"create APIExportEndpointSlice when APIBindings present": {
129134
secretRefSet: true,
130135
secretExists: true,
131136

132-
wantStatusHashSet: true,
133-
wantIdentityValid: true,
137+
wantStatusHashSet: true,
138+
apiExportEndpointSliceNotFound: true,
139+
wantCreateAPIExportEndpointSlice: true,
140+
wantIdentityValid: true,
141+
},
142+
"skip APIExportEndpointSlice creation when skip annotation is present": {
143+
secretRefSet: true,
144+
secretExists: true,
134145

135-
apiBindings: []interface{}{
136-
"something",
137-
},
138-
wantVirtualWorkspaceURLsReady: true,
146+
wantStatusHashSet: true,
147+
apiExportEndpointSliceNotFound: true,
148+
wantCreateAPIExportEndpointSlice: false,
149+
wantIdentityValid: true,
150+
skipEndpointSliceAnnotation: true,
151+
},
152+
"virtual workspace URLs when feature gate enabled": {
153+
secretRefSet: true,
154+
secretExists: true,
155+
156+
wantStatusHashSet: true,
157+
wantIdentityValid: true,
158+
wantVirtualWorkspaceURLs: true,
159+
},
160+
"error listing shards with feature gate enabled": {
161+
secretRefSet: true,
162+
secretExists: true,
163+
164+
wantStatusHashSet: true,
165+
wantIdentityValid: true,
166+
wantVirtualWorkspaceURLs: false,
167+
listShardsError: errors.New("foo"),
139168
},
140169
}
141170

142171
for name, tc := range tests {
143172
t.Run(name, func(t *testing.T) {
173+
// Skip virtual workspace URL tests if feature gate is not enabled
174+
if tc.wantVirtualWorkspaceURLs && !originalFeatureGate {
175+
t.Skip("Skipping test that requires EnableDeprecatedAPIExportVirtualWorkspacesUrls feature gate")
176+
}
177+
144178
createSecretCalled := false
179+
createEndpointSliceCalled := false
145180

146181
expectedKey := "abc"
147182
expectedHash := fmt.Sprintf("%x", sha256.Sum256([]byte(expectedKey)))
@@ -155,6 +190,16 @@ func TestReconcile(t *testing.T) {
155190
return nil
156191
},
157192
secretNamespace: "default-ns",
193+
getAPIExportEndpointSlice: func(clusterName logicalcluster.Name, name string) (*apisv1alpha1.APIExportEndpointSlice, error) {
194+
if tc.apiExportEndpointSliceNotFound {
195+
return nil, apierrors.NewNotFound(corev1.Resource("apiexportendpointslices"), name)
196+
}
197+
return &apisv1alpha1.APIExportEndpointSlice{}, nil
198+
},
199+
createAPIExportEndpointSlice: func(ctx context.Context, clusterName logicalcluster.Path, apiExportEndpointSlice *apisv1alpha1.APIExportEndpointSlice) error {
200+
createEndpointSliceCalled = true
201+
return nil
202+
},
158203
getSecret: func(ctx context.Context, clusterName logicalcluster.Name, ns, name string) (*corev1.Secret, error) {
159204
if tc.secretExists {
160205
secret := &corev1.Secret{
@@ -237,6 +282,10 @@ func TestReconcile(t *testing.T) {
237282
conditions.MarkFalse(apiExport, apisv1alpha2.APIExportIdentityValid, apisv1alpha2.IdentityVerificationFailedReason, conditionsv1alpha1.ConditionSeverityError, "")
238283
}
239284

285+
if tc.skipEndpointSliceAnnotation {
286+
apiExport.Annotations[apisv1alpha2.APIExportEndpointSliceSkipAnnotation] = "true"
287+
}
288+
240289
err := c.reconcile(context.Background(), apiExport)
241290
if tc.wantError {
242291
require.Error(t, err, "expected an error")
@@ -288,19 +337,18 @@ func TestReconcile(t *testing.T) {
288337
requireConditionMatches(t, apiExport, conditions.TrueCondition(apisv1alpha2.APIExportIdentityValid))
289338
}
290339

291-
if tc.wantVirtualWorkspaceURLsError {
292-
requireConditionMatches(t, apiExport,
293-
conditions.FalseCondition(
294-
apisv1alpha2.APIExportVirtualWorkspaceURLsReady,
295-
apisv1alpha2.ErrorGeneratingURLsReason,
296-
conditionsv1alpha1.ConditionSeverityError,
297-
"",
298-
),
299-
)
300-
}
340+
require.Equal(t, tc.wantCreateAPIExportEndpointSlice, createEndpointSliceCalled, "expected createEndpointSliceCalled to be %v", tc.wantCreateAPIExportEndpointSlice)
301341

302-
if tc.wantVirtualWorkspaceURLsReady {
303-
requireConditionMatches(t, apiExport, conditions.TrueCondition(apisv1alpha2.APIExportVirtualWorkspaceURLsReady))
342+
if tc.wantVirtualWorkspaceURLs {
343+
//nolint:staticcheck
344+
require.NotNil(t, apiExport.Status.VirtualWorkspaces, "expected virtual workspace URLs to be set")
345+
//nolint:staticcheck
346+
require.Len(t, apiExport.Status.VirtualWorkspaces, 2, "expected 2 virtual workspace URLs")
347+
require.True(t, conditions.IsTrue(apiExport, apisv1alpha2.APIExportVirtualWorkspaceURLsReady), "expected virtual workspace URLs to be ready")
348+
} else {
349+
//nolint:staticcheck
350+
require.Nil(t, apiExport.Status.VirtualWorkspaces, "expected virtual workspace URLs to be nil")
351+
require.False(t, conditions.Has(apiExport, apisv1alpha2.APIExportVirtualWorkspaceURLsReady), "expected virtual workspace URLs condition to not exist")
304352
}
305353
})
306354
}

‎pkg/reconciler/apis/apiexport/apiexport_reconcile.go

Lines changed: 53 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -31,10 +31,12 @@ import (
3131
"github.com/kcp-dev/logicalcluster/v3"
3232

3333
virtualworkspacesoptions "github.com/kcp-dev/kcp/cmd/virtual-workspaces/options"
34+
kcpfeatures "github.com/kcp-dev/kcp/pkg/features"
3435
"github.com/kcp-dev/kcp/pkg/logging"
3536
apiexportbuilder "github.com/kcp-dev/kcp/pkg/virtual/apiexport/builder"
3637
apisv1alpha1 "github.com/kcp-dev/kcp/sdk/apis/apis/v1alpha1"
3738
apisv1alpha2 "github.com/kcp-dev/kcp/sdk/apis/apis/v1alpha2"
39+
"github.com/kcp-dev/kcp/sdk/apis/core"
3840
conditionsv1alpha1 "github.com/kcp-dev/kcp/sdk/apis/third_party/conditions/apis/conditions/v1alpha1"
3941
"github.com/kcp-dev/kcp/sdk/apis/third_party/conditions/util/conditions"
4042
)
@@ -46,6 +48,7 @@ func (c *controller) reconcile(ctx context.Context, apiExport *apisv1alpha2.APIE
4648
}
4749

4850
clusterName := logicalcluster.From(apiExport)
51+
clusterPath := apiExport.Annotations[core.LogicalClusterPathAnnotationKey]
4952

5053
if identity.SecretRef == nil {
5154
c.ensureSecretNamespaceExists(ctx, clusterName)
@@ -97,29 +100,58 @@ func (c *controller) reconcile(ctx context.Context, apiExport *apisv1alpha2.APIE
97100
)
98101
}
99102

100-
// TODO(sttts): reactivate this with multi-shard support eventually
101-
/*
102-
// check if any APIBindings are bound to this APIExport. If so, add a virtualworkspaceURL
103-
apiBindings, err := c.getAPIBindingsForAPIExport(clusterName, apiExport.Name)
103+
// Ensure the APIExportEndpointSlice exists
104+
if _, ok := apiExport.Annotations[apisv1alpha2.APIExportEndpointSliceSkipAnnotation]; !ok {
105+
_, err := c.getAPIExportEndpointSlice(clusterName, apiExport.Name)
104106
if err != nil {
105-
return fmt.Errorf("error checking for APIBindings with APIExport %s|%s: %w", clusterName, apiExport.Name, err)
107+
if errors.IsNotFound(err) {
108+
// Create the APIExportEndpointSlice
109+
apiExportEndpointSlice := &apisv1alpha1.APIExportEndpointSlice{
110+
ObjectMeta: metav1.ObjectMeta{
111+
Name: apiExport.Name,
112+
OwnerReferences: []metav1.OwnerReference{
113+
{
114+
APIVersion: apisv1alpha1.SchemeGroupVersion.String(),
115+
Kind: "APIExport",
116+
Name: apiExport.Name,
117+
UID: apiExport.UID,
118+
},
119+
},
120+
},
121+
Spec: apisv1alpha1.APIExportEndpointSliceSpec{
122+
APIExport: apisv1alpha1.ExportBindingReference{
123+
Name: apiExport.Name,
124+
Path: clusterPath,
125+
},
126+
},
127+
}
128+
if err := c.createAPIExportEndpointSlice(ctx, clusterName.Path(), apiExportEndpointSlice); err != nil {
129+
return fmt.Errorf("error creating APIExportEndpointSlice for APIExport %s|%s: %w", clusterName, apiExport.Name, err)
130+
}
131+
} else {
132+
return fmt.Errorf("error getting APIExportEndpointSlice for APIExport %s|%s: %w", clusterName, apiExport.Name, err)
133+
}
106134
}
135+
}
107136

108-
// If there are no bindings, then we can't create a URL yet.
109-
if len(apiBindings) == 0 {
110-
return nil
137+
// Ensure the APIExport has a virtual workspace URL
138+
// TODO(mjudeikis): Remove this once we remove feature gate.
139+
if kcpfeatures.DefaultFeatureGate.Enabled(kcpfeatures.EnableDeprecatedAPIExportVirtualWorkspacesUrls) {
140+
if err := c.updateVirtualWorkspaceURLs(ctx, apiExport); err != nil {
141+
conditions.MarkFalse(
142+
apiExport,
143+
apisv1alpha2.APIExportVirtualWorkspaceURLsReady,
144+
apisv1alpha2.ErrorGeneratingURLsReason,
145+
conditionsv1alpha1.ConditionSeverityError,
146+
"%v",
147+
err,
148+
)
111149
}
112-
*/
113-
114-
if err := c.updateVirtualWorkspaceURLs(ctx, apiExport); err != nil {
115-
conditions.MarkFalse(
116-
apiExport,
117-
apisv1alpha2.APIExportVirtualWorkspaceURLsReady,
118-
apisv1alpha2.ErrorGeneratingURLsReason,
119-
conditionsv1alpha1.ConditionSeverityError,
120-
"%v",
121-
err,
122-
)
150+
} else {
151+
// Remove the condition and status.virtualWorkspaces if the feature gate is disabled.
152+
conditions.Delete(apiExport, apisv1alpha2.APIExportVirtualWorkspaceURLsReady)
153+
//nolint:staticcheck
154+
apiExport.Status.VirtualWorkspaces = nil
123155
}
124156

125157
return nil
@@ -216,11 +248,11 @@ func (c *controller) updateVirtualWorkspaceURLs(ctx context.Context, apiExport *
216248
desiredURLs.Insert(u.String())
217249
}
218250

219-
//nolint:staticcheck // SA1019 VirtualWorkspaces is deprecated but not removed yet
251+
//nolint:staticcheck
220252
apiExport.Status.VirtualWorkspaces = nil
221253

222254
for _, u := range sets.List[string](desiredURLs) {
223-
//nolint:staticcheck // SA1019 VirtualWorkspaces is deprecated but not removed yet
255+
//nolint:staticcheck
224256
apiExport.Status.VirtualWorkspaces = append(apiExport.Status.VirtualWorkspaces, apisv1alpha2.VirtualWorkspace{
225257
URL: u,
226258
})

‎pkg/server/controllers.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1050,6 +1050,7 @@ func (s *Server) installAPIExportController(ctx context.Context, config *rest.Co
10501050
c, err := apiexport.NewController(
10511051
kcpClusterClient,
10521052
s.KcpSharedInformerFactory.Apis().V1alpha2().APIExports(),
1053+
s.KcpSharedInformerFactory.Apis().V1alpha1().APIExportEndpointSlices(),
10531054
s.CacheKcpSharedInformerFactory.Core().V1alpha1().Shards(),
10541055
kubeClusterClient,
10551056
s.KubeSharedInformerFactory.Core().V1().Namespaces(),
@@ -1068,6 +1069,7 @@ func (s *Server) installAPIExportController(ctx context.Context, config *rest.Co
10681069
return wait.PollUntilContextCancel(ctx, waitPollInterval, true, func(ctx context.Context) (bool, error) {
10691070
return s.ApiExtensionsSharedInformerFactory.Apiextensions().V1().CustomResourceDefinitions().Informer().HasSynced() &&
10701071
s.KcpSharedInformerFactory.Apis().V1alpha2().APIExports().Informer().HasSynced() &&
1072+
s.KcpSharedInformerFactory.Apis().V1alpha1().APIExportEndpointSlices().Informer().HasSynced() &&
10711073
s.KubeSharedInformerFactory.Core().V1().Namespaces().Informer().HasSynced() &&
10721074
s.KubeSharedInformerFactory.Core().V1().Secrets().Informer().HasSynced(), nil
10731075
})

‎pkg/server/server.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -439,6 +439,7 @@ func (s *Server) Run(ctx context.Context) error {
439439
logger.Info("finished bootstrapping the shard workspace")
440440

441441
go s.KcpSharedInformerFactory.Apis().V1alpha2().APIExports().Informer().Run(hookContext.Done())
442+
go s.KcpSharedInformerFactory.Apis().V1alpha1().APIExportEndpointSlices().Informer().Run(hookContext.Done())
442443
go s.CacheKcpSharedInformerFactory.Apis().V1alpha2().APIExports().Informer().Run(hookContext.Done())
443444
go s.KcpSharedInformerFactory.Core().V1alpha1().LogicalClusters().Informer().Run(hookContext.Done())
444445
go s.KcpSharedInformerFactory.Cache().V1alpha1().CachedResources().Informer().Run(hookContext.Done())

0 commit comments

Comments
 (0)