Skip to content

Commit 61f55fc

Browse files
joejstuartjoejstuart
committed
Format imageUrl to accept multiple sub-paths
Repo urls can have multiple orgs. This new regex supports this and also drops support for https. The component URL does not contain the https in ec-cli, so this simplifies things. https://issues.redhat.com/browse/EC-1258
1 parent 45e50ef commit 61f55fc

5 files changed

Lines changed: 138 additions & 6 deletions

‎api/config/appstudio.redhat.com_enterprisecontractpolicies.yaml‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,7 @@ spec:
177177
type: string
178178
imageUrl:
179179
description: ImageUrl is used to specify an image by its URL without a tag.
180-
pattern: ^(?:https:\/\/)?[a-z0-9.-]+\/[a-z0-9-]+\/[a-z0-9-]+$
180+
pattern: ^[a-z0-9][a-z0-9.-]*[a-z0-9](?:\/[a-z0-9][a-z0-9-]*[a-z0-9]){2,}$
181181
type: string
182182
value:
183183
type: string
@@ -210,7 +210,7 @@ spec:
210210
type: string
211211
imageUrl:
212212
description: ImageUrl is used to specify an image by its URL without a tag.
213-
pattern: ^(?:https:\/\/)?[a-z0-9.-]+\/[a-z0-9-]+\/[a-z0-9-]+$
213+
pattern: ^[a-z0-9][a-z0-9.-]*[a-z0-9](?:\/[a-z0-9][a-z0-9-]*[a-z0-9]){2,}$
214214
type: string
215215
value:
216216
type: string

‎api/v1alpha1/enterprisecontractpolicy_types.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ type VolatileCriteria struct {
113113

114114
// ImageUrl is used to specify an image by its URL without a tag.
115115
// +optional
116-
// +kubebuilder:validation:Pattern=`^(?:https:\/\/)?[a-z0-9.-]+\/[a-z0-9-]+\/[a-z0-9-]+$`
116+
// +kubebuilder:validation:Pattern=`^[a-z0-9][a-z0-9.-]*[a-z0-9](?:\/[a-z0-9][a-z0-9-]*[a-z0-9]){2,}$`
117117
ImageUrl string `json:"imageUrl,omitempty"`
118118
}
119119

‎api/v1alpha1/enterprisecontractpolicy_types_test.go‎

Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,3 +149,135 @@ func TestMultiplevolatileConfigWithSameValue(t *testing.T) {
149149
t.Errorf("did not expect validation errors: %v", errs)
150150
}
151151
}
152+
153+
func TestImageUrlPattern(t *testing.T) {
154+
tests := []struct {
155+
name string
156+
url string
157+
wantValid bool
158+
omitField bool // true if the field should be omitted entirely
159+
}{
160+
// Valid cases
161+
{"Simple registry path", "quay.io/org/repo1", true, false},
162+
{"Multi-level org path", "registry.io/org1/org2/repo1", true, false},
163+
{"Docker library path", "docker.io/library/nginx", true, false},
164+
{"GitHub container registry", "ghcr.io/org/project/repo", true, false},
165+
{"Registry with subdomain", "my-registry.com/org/suborg/repo", true, false},
166+
{"Registry with multiple subdomains", "prod.registry.example.com/org/repo", true, false},
167+
{"Registry with hyphens", "my-registry.example.com/org-name/repo-name", true, false},
168+
{"Registry with numbers", "registry123.example.com/org123/repo123", true, false},
169+
{"Extra path component", "registry/org/repo/extra", true, false},
170+
{"Omitted field", "", true, true}, // Field is omitted entirely
171+
172+
// Invalid cases
173+
{"URL with HTTPS", "https://quay.io/org/repo", false, false},
174+
{"Localhost with port", "localhost:5000/org/repo", false, false},
175+
{"Invalid character @", "invalid@registry/org/repo", false, false},
176+
{"Missing repo", "registry/org", false, false},
177+
{"Double slash", "registry//org/repo", false, false},
178+
{"Trailing slash", "registry/org/repo/", false, false},
179+
{"With tag", "registry/org/repo:tag", false, false},
180+
{"With digest", "registry/org/repo@sha256:abc123", false, false},
181+
{"Missing repo with slash", "registry/org/", false, false},
182+
{"Only registry with slash", "registry/", false, false},
183+
{"Only registry", "registry", false, false},
184+
{"Extra path with slash", "registry/org/repo/extra/", false, false},
185+
}
186+
187+
for _, tt := range tests {
188+
t.Run(tt.name, func(t *testing.T) {
189+
// Create a policy with the test URL
190+
policy := EnterpriseContractPolicy{
191+
Spec: EnterpriseContractPolicySpec{
192+
Sources: []Source{
193+
{
194+
VolatileConfig: &VolatileSourceConfig{
195+
Exclude: []VolatileCriteria{
196+
{
197+
Value: "test-rule",
198+
},
199+
},
200+
},
201+
},
202+
},
203+
},
204+
}
205+
if !tt.omitField {
206+
policy.Spec.Sources[0].VolatileConfig.Exclude[0].ImageUrl = tt.url
207+
}
208+
209+
// Create a CRD validation schema
210+
crd := v1.CustomResourceDefinition{}
211+
bytes, err := os.ReadFile("../../config/crd/bases/appstudio.redhat.com_enterprisecontractpolicies.yaml")
212+
if err != nil {
213+
t.Fatalf("unexpected error reading CRD: %s", err)
214+
}
215+
if err := yaml.Unmarshal(bytes, &crd); err != nil {
216+
t.Fatalf("unexpected error when decoding schema: %s", err)
217+
}
218+
219+
crdv := apiextensions.CustomResourceValidation{}
220+
if err := v1.Convert_v1_CustomResourceValidation_To_apiextensions_CustomResourceValidation(crd.Spec.Versions[0].Schema, &crdv, nil); err != nil {
221+
t.Fatalf("failed in CRD validation conversion: %s", err)
222+
}
223+
224+
s, err := schema.NewStructural(crdv.OpenAPIV3Schema)
225+
if err != nil {
226+
t.Fatalf("unexpected error when creating structural: %s", err)
227+
}
228+
229+
v := validation.NewSchemaValidatorFromOpenAPI(s.ToKubeOpenAPI())
230+
231+
// Convert policy to unstructured for validation
232+
obj := unstructured.Unstructured{}
233+
obj.SetUnstructuredContent(map[string]interface{}{
234+
"apiVersion": "appstudio.redhat.com/v1alpha1",
235+
"kind": "EnterpriseContractPolicy",
236+
"spec": map[string]interface{}{
237+
"sources": []interface{}{
238+
map[string]interface{}{
239+
"volatileConfig": map[string]interface{}{
240+
"exclude": []interface{}{
241+
func() map[string]interface{} {
242+
m := map[string]interface{}{
243+
"value": "test-rule",
244+
}
245+
if !tt.omitField {
246+
m["imageUrl"] = tt.url
247+
}
248+
return m
249+
}(),
250+
},
251+
},
252+
},
253+
},
254+
},
255+
})
256+
257+
// Validate the object
258+
result := v.Validate(&obj)
259+
isValid := result.IsValid()
260+
261+
if isValid != tt.wantValid {
262+
t.Errorf("Validation for %q = %v, want %v. Errors: %v", tt.url, isValid, tt.wantValid, result.Errors)
263+
}
264+
265+
// Also validate the actual policy object
266+
policyObj := unstructured.Unstructured{}
267+
policyBytes, err := json.Marshal(policy)
268+
if err != nil {
269+
t.Fatalf("unexpected error marshaling policy: %s", err)
270+
}
271+
if err := json.Unmarshal(policyBytes, &policyObj.Object); err != nil {
272+
t.Fatalf("unexpected error unmarshaling policy: %s", err)
273+
}
274+
275+
policyResult := v.Validate(&policyObj)
276+
policyIsValid := policyResult.IsValid()
277+
278+
if policyIsValid != tt.wantValid {
279+
t.Errorf("Policy validation for %q = %v, want %v. Errors: %v", tt.url, policyIsValid, tt.wantValid, policyResult.Errors)
280+
}
281+
})
282+
}
283+
}

‎api/v1alpha1/policy_spec.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -178,7 +178,7 @@
178178
},
179179
"imageUrl": {
180180
"type": "string",
181-
"description": "ImageUrl is used to specify an image by its URL without a tag.\n+optional\n+kubebuilder:validation:Pattern=`^(?:https:\\/\\/)?[a-z0-9.-]+\\/[a-z0-9-]+\\/[a-z0-9-]+$`"
181+
"description": "ImageUrl is used to specify an image by its URL without a tag.\n+optional\n+kubebuilder:validation:Pattern=`^[a-z0-9][a-z0-9.-]*[a-z0-9](?:\\/[a-z0-9][a-z0-9-]*[a-z0-9]){2,}$`"
182182
}
183183
},
184184
"additionalProperties": false,

‎config/crd/bases/appstudio.redhat.com_enterprisecontractpolicies.yaml‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,7 @@ spec:
177177
type: string
178178
imageUrl:
179179
description: ImageUrl is used to specify an image by its URL without a tag.
180-
pattern: ^(?:https:\/\/)?[a-z0-9.-]+\/[a-z0-9-]+\/[a-z0-9-]+$
180+
pattern: ^[a-z0-9][a-z0-9.-]*[a-z0-9](?:\/[a-z0-9][a-z0-9-]*[a-z0-9]){2,}$
181181
type: string
182182
value:
183183
type: string
@@ -210,7 +210,7 @@ spec:
210210
type: string
211211
imageUrl:
212212
description: ImageUrl is used to specify an image by its URL without a tag.
213-
pattern: ^(?:https:\/\/)?[a-z0-9.-]+\/[a-z0-9-]+\/[a-z0-9-]+$
213+
pattern: ^[a-z0-9][a-z0-9.-]*[a-z0-9](?:\/[a-z0-9][a-z0-9-]*[a-z0-9]){2,}$
214214
type: string
215215
value:
216216
type: string

0 commit comments

Comments
 (0)