Skip to content

Commit 4d6382d

Browse files
fix(subscription): allow empty plan name in external-name validation (#639)
1 parent 00e16bc commit 4d6382d

2 files changed

Lines changed: 25 additions & 10 deletions

File tree

internal/controller/account/subscription/subscription.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -249,19 +249,20 @@ func (c *external) loadSubscription(ctx context.Context, cr *v1alpha1.Subscripti
249249
return nil, nil
250250
}
251251

252-
// Validate external-name format (should be appName/planName)
252+
// Validate external-name format (should be appName/planName, planName may be empty)
253253
if !isValidExternalNameFormat(externalName) {
254-
return nil, errors.Errorf("invalid external-name format: %s, expected format: <appName>/<planName>", externalName)
254+
return nil, errors.Errorf("invalid external-name format: %s, expected format: <appName>/<planName> (planName may be empty)", externalName)
255255
}
256256

257257
// Get subscription from API - if it returns nil, it means resource doesn't exist (drift scenario)
258258
return c.apiHandler.GetSubscription(ctx, externalName)
259259
}
260260

261261
// isValidExternalNameFormat validates that the external name is in the format appName/planName
262+
// planName may be empty
262263
func isValidExternalNameFormat(externalName string) bool {
263264
parts := strings.Split(externalName, "/")
264-
return len(parts) == 2 && parts[0] != "" && parts[1] != ""
265+
return len(parts) == 2 && parts[0] != ""
265266
}
266267

267268
// syncStatus delegates saving the observation based on external resource to the typemapper

internal/controller/account/subscription/subscription_test.go

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ func TestObserve(t *testing.T) {
6767
},
6868
want: want{
6969
o: managed.ExternalObservation{},
70-
err: errors.Wrap(errors.New("invalid external-name format: invalid-format-without-slash, expected format: <appName>/<planName>"), "while loading subscription"),
70+
err: errors.Wrap(errors.New("invalid external-name format: invalid-format-without-slash, expected format: <appName>/<planName> (planName may be empty)"), "while loading subscription"),
7171
cr: NewSubscription("sub-test", WithStatus(v1alpha1.SubscriptionObservation{}), WithExternalName("invalid-format-without-slash")),
7272
},
7373
},
@@ -78,19 +78,33 @@ func TestObserve(t *testing.T) {
7878
},
7979
want: want{
8080
o: managed.ExternalObservation{},
81-
err: errors.Wrap(errors.New("invalid external-name format: /, expected format: <appName>/<planName>"), "while loading subscription"),
81+
err: errors.Wrap(errors.New("invalid external-name format: /, expected format: <appName>/<planName> (planName may be empty)"), "while loading subscription"),
8282
cr: NewSubscription("sub-test", WithStatus(v1alpha1.SubscriptionObservation{}), WithExternalName("/")),
8383
},
8484
},
85-
"InvalidExternalNameFormatEmptyParts": {
86-
reason: "Invalid external-name format with empty parts should return error",
85+
"InvalidExternalNameFormatEmptyAppName": {
86+
reason: "Invalid external-name format with empty app name should return error",
8787
args: args{
88-
cr: NewSubscription("sub-test", WithStatus(v1alpha1.SubscriptionObservation{}), WithExternalName("appname/")),
88+
cr: NewSubscription("sub-test", WithStatus(v1alpha1.SubscriptionObservation{}), WithExternalName("/planName")),
8989
},
9090
want: want{
9191
o: managed.ExternalObservation{},
92-
err: errors.Wrap(errors.New("invalid external-name format: appname/, expected format: <appName>/<planName>"), "while loading subscription"),
93-
cr: NewSubscription("sub-test", WithStatus(v1alpha1.SubscriptionObservation{}), WithExternalName("appname/")),
92+
err: errors.Wrap(errors.New("invalid external-name format: /planName, expected format: <appName>/<planName> (planName may be empty)"), "while loading subscription"),
93+
cr: NewSubscription("sub-test", WithStatus(v1alpha1.SubscriptionObservation{}), WithExternalName("/planName")),
94+
},
95+
},
96+
"ValidExternalNameFormatEmptyPlanName": {
97+
reason: "External-name format with empty plan name should be valid",
98+
args: args{
99+
cr: NewSubscription("sub-test", WithStatus(v1alpha1.SubscriptionObservation{}), WithExternalName("appname/")),
100+
mockApiHandler: &MockApiHandler{
101+
returnGet: nil,
102+
returnErr: nil,
103+
},
104+
},
105+
want: want{
106+
o: managed.ExternalObservation{ResourceExists: false},
107+
cr: NewSubscription("sub-test", WithStatus(v1alpha1.SubscriptionObservation{}), WithExternalName("appname/")),
94108
},
95109
},
96110
"APIErrorOnRead": {

0 commit comments

Comments
 (0)