Skip to content

Commit fd0b98b

Browse files
mjudeikismjudeikis
committed
Add APIExport annotation to skip APIExportEndpointSlice creation
1 parent 2bba901 commit fd0b98b

File tree

3 files changed

+97
-19
lines changed

3 files changed

+97
-19
lines changed

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

Lines changed: 58 additions & 0 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
@@ -50,6 +54,7 @@ func TestReconcile(t *testing.T) {
5054
hasPreexistingVerifyFailure bool
5155
listShardsError error
5256
apiExportEndpointSliceNotFound bool
57+
skipEndpointSliceAnnotation bool
5358

5459
apiBindings []interface{}
5560

@@ -62,6 +67,7 @@ func TestReconcile(t *testing.T) {
6267
wantVerifyFailure bool
6368
wantIdentityValid bool
6469
wantCreateAPIExportEndpointSlice bool
70+
wantVirtualWorkspaceURLs bool
6571
}{
6672
"create secret when ref is nil and secret doesn't exist": {
6773
secretExists: false,
@@ -133,11 +139,44 @@ func TestReconcile(t *testing.T) {
133139
wantCreateAPIExportEndpointSlice: true,
134140
wantIdentityValid: true,
135141
},
142+
"skip APIExportEndpointSlice creation when skip annotation is present": {
143+
secretRefSet: true,
144+
secretExists: true,
145+
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"),
168+
},
136169
}
137170

138171
for name, tc := range tests {
139172
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+
140178
createSecretCalled := false
179+
createEndpointSliceCalled := false
141180

142181
expectedKey := "abc"
143182
expectedHash := fmt.Sprintf("%x", sha256.Sum256([]byte(expectedKey)))
@@ -158,6 +197,7 @@ func TestReconcile(t *testing.T) {
158197
return &apisv1alpha1.APIExportEndpointSlice{}, nil
159198
},
160199
createAPIExportEndpointSlice: func(ctx context.Context, clusterName logicalcluster.Path, apiExportEndpointSlice *apisv1alpha1.APIExportEndpointSlice) error {
200+
createEndpointSliceCalled = true
161201
return nil
162202
},
163203
getSecret: func(ctx context.Context, clusterName logicalcluster.Name, ns, name string) (*corev1.Secret, error) {
@@ -242,6 +282,10 @@ func TestReconcile(t *testing.T) {
242282
conditions.MarkFalse(apiExport, apisv1alpha2.APIExportIdentityValid, apisv1alpha2.IdentityVerificationFailedReason, conditionsv1alpha1.ConditionSeverityError, "")
243283
}
244284

285+
if tc.skipEndpointSliceAnnotation {
286+
apiExport.Annotations[apisv1alpha2.APIExportEndpointSliceSkipAnnotation] = "true"
287+
}
288+
245289
err := c.reconcile(context.Background(), apiExport)
246290
if tc.wantError {
247291
require.Error(t, err, "expected an error")
@@ -292,6 +336,20 @@ func TestReconcile(t *testing.T) {
292336
if tc.wantIdentityValid {
293337
requireConditionMatches(t, apiExport, conditions.TrueCondition(apisv1alpha2.APIExportIdentityValid))
294338
}
339+
340+
require.Equal(t, tc.wantCreateAPIExportEndpointSlice, createEndpointSliceCalled, "expected createEndpointSliceCalled to be %v", tc.wantCreateAPIExportEndpointSlice)
341+
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")
352+
}
295353
})
296354
}
297355
}

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

Lines changed: 34 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -101,31 +101,41 @@ func (c *controller) reconcile(ctx context.Context, apiExport *apisv1alpha2.APIE
101101
}
102102

103103
// Ensure the APIExportEndpointSlice exists
104-
_, err := c.getAPIExportEndpointSlice(clusterName, apiExport.Name)
105-
if err != nil {
106-
if errors.IsNotFound(err) {
107-
// Create the APIExportEndpointSlice
108-
apiExportEndpointSlice := &apisv1alpha1.APIExportEndpointSlice{
109-
ObjectMeta: metav1.ObjectMeta{
110-
Name: apiExport.Name,
111-
},
112-
Spec: apisv1alpha1.APIExportEndpointSliceSpec{
113-
APIExport: apisv1alpha1.ExportBindingReference{
104+
if _, ok := apiExport.Annotations[apisv1alpha2.APIExportEndpointSliceSkipAnnotation]; !ok {
105+
_, err := c.getAPIExportEndpointSlice(clusterName, apiExport.Name)
106+
if err != nil {
107+
if errors.IsNotFound(err) {
108+
// Create the APIExportEndpointSlice
109+
apiExportEndpointSlice := &apisv1alpha1.APIExportEndpointSlice{
110+
ObjectMeta: metav1.ObjectMeta{
114111
Name: apiExport.Name,
115-
Path: clusterPath,
112+
OwnerReferences: []metav1.OwnerReference{
113+
{
114+
APIVersion: apisv1alpha1.SchemeGroupVersion.String(),
115+
Kind: "APIExport",
116+
Name: apiExport.Name,
117+
UID: apiExport.UID,
118+
},
119+
},
116120
},
117-
},
118-
}
119-
if err := c.createAPIExportEndpointSlice(ctx, clusterName.Path(), apiExportEndpointSlice); err != nil {
120-
return fmt.Errorf("error creating APIExportEndpointSlice for APIExport %s|%s: %w", clusterName, apiExport.Name, err)
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)
121133
}
122-
} else {
123-
return fmt.Errorf("error getting APIExportEndpointSlice for APIExport %s|%s: %w", clusterName, apiExport.Name, err)
124134
}
125135
}
126136

127-
// Ensure the APIExportEndpointSlice has a virtual workspace URL
128-
// TODO(mjudeikis): Remove this and move to batteries.
137+
// Ensure the APIExport has a virtual workspace URL
138+
// TODO(mjudeikis): Remove this once we remove feature gate.
129139
if kcpfeatures.DefaultFeatureGate.Enabled(kcpfeatures.EnableDeprecatedAPIExportVirtualWorkspacesUrls) {
130140
if err := c.updateVirtualWorkspaceURLs(ctx, apiExport); err != nil {
131141
conditions.MarkFalse(
@@ -137,6 +147,11 @@ func (c *controller) reconcile(ctx context.Context, apiExport *apisv1alpha2.APIE
137147
err,
138148
)
139149
}
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
140155
}
141156

142157
return nil

‎sdk/apis/apis/v1alpha2/types_apiexport.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,11 @@ import (
2626
conditionsv1alpha1 "github.com/kcp-dev/kcp/sdk/apis/third_party/conditions/apis/conditions/v1alpha1"
2727
)
2828

29+
const (
30+
// APIExportEndpointSliceSkipAnnotation is an annotation that can be set on an APIExport to skip the creation of default APIExportEndpointSlice.
31+
APIExportEndpointSliceSkipAnnotation = "apiexports.apis.kcp.io/skip-endpointslice"
32+
)
33+
2934
// These are valid conditions of APIExport.
3035
const (
3136
APIExportIdentityValid conditionsv1alpha1.ConditionType = "IdentityValid"

0 commit comments

Comments
 (0)