diff --git a/api/config/appstudio.redhat.com_enterprisecontractpolicies.yaml b/api/config/appstudio.redhat.com_enterprisecontractpolicies.yaml index ccdfef3..60d1065 100644 --- a/api/config/appstudio.redhat.com_enterprisecontractpolicies.yaml +++ b/api/config/appstudio.redhat.com_enterprisecontractpolicies.yaml @@ -159,6 +159,16 @@ spec: items: description: VolatileCriteria includes or excludes a policy rule with effective dates as an option. properties: + componentNames: + description: |- + ComponentNames is used to specify component names from + ApplicationSnapshot. This allows filtering in scenarios where + multiple components share the same image repository. + items: + minLength: 1 + type: string + type: array + x-kubernetes-list-type: set effectiveOn: format: date-time type: string @@ -187,6 +197,9 @@ spec: required: - value type: object + x-kubernetes-validations: + - message: only one of imageUrl, imageDigest, imageRef, or componentNames may be set + rule: '(has(self.imageUrl) ? 1 : 0) + (has(self.imageDigest) ? 1 : 0) + (has(self.imageRef) ? 1 : 0) + (has(self.componentNames) ? 1 : 0) <= 1' type: array include: description: |- @@ -195,6 +208,16 @@ spec: items: description: VolatileCriteria includes or excludes a policy rule with effective dates as an option. properties: + componentNames: + description: |- + ComponentNames is used to specify component names from + ApplicationSnapshot. This allows filtering in scenarios where + multiple components share the same image repository. + items: + minLength: 1 + type: string + type: array + x-kubernetes-list-type: set effectiveOn: format: date-time type: string @@ -223,6 +246,9 @@ spec: required: - value type: object + x-kubernetes-validations: + - message: only one of imageUrl, imageDigest, imageRef, or componentNames may be set + rule: '(has(self.imageUrl) ? 1 : 0) + (has(self.imageDigest) ? 1 : 0) + (has(self.imageRef) ? 1 : 0) + (has(self.componentNames) ? 1 : 0) <= 1' type: array type: object type: object diff --git a/api/v1alpha1/enterprisecontractpolicy_types.go b/api/v1alpha1/enterprisecontractpolicy_types.go index 3e93727..8706bdf 100644 --- a/api/v1alpha1/enterprisecontractpolicy_types.go +++ b/api/v1alpha1/enterprisecontractpolicy_types.go @@ -90,7 +90,11 @@ type SourceConfig struct { Include []string `json:"include,omitempty"` } +// +kubebuilder:validation:MinLength=1 +type ComponentName string + // VolatileCriteria includes or excludes a policy rule with effective dates as an option. +// +kubebuilder:validation:XValidation:rule="(has(self.imageUrl) ? 1 : 0) + (has(self.imageDigest) ? 1 : 0) + (has(self.imageRef) ? 1 : 0) + (has(self.componentNames) ? 1 : 0) <= 1",message="only one of imageUrl, imageDigest, imageRef, or componentNames may be set" type VolatileCriteria struct { Value string `json:"value"` // +optional @@ -116,6 +120,13 @@ type VolatileCriteria struct { // +kubebuilder:validation:Pattern=`^[a-z0-9][a-z0-9.-]*[a-z0-9](?:\/[a-z0-9][a-z0-9-]*[a-z0-9]){2,}$` ImageUrl string `json:"imageUrl,omitempty"` + // ComponentNames is used to specify component names from + // ApplicationSnapshot. This allows filtering in scenarios where + // multiple components share the same image repository. + // +optional + // +listType=set + ComponentNames []ComponentName `json:"componentNames,omitempty"` + // Reference is used to include a link to related information such as a Jira issue URL. // +optional Reference string `json:"reference,omitempty"` diff --git a/api/v1alpha1/enterprisecontractpolicy_types_test.go b/api/v1alpha1/enterprisecontractpolicy_types_test.go index 8634d32..568fa7b 100644 --- a/api/v1alpha1/enterprisecontractpolicy_types_test.go +++ b/api/v1alpha1/enterprisecontractpolicy_types_test.go @@ -395,3 +395,234 @@ func TestReferenceField(t *testing.T) { }) } } + +func TestVolatileCriteriaMutualExclusivity(t *testing.T) { + tests := []struct { + name string + imageUrl string + imageDigest string + imageRef string + componentNames []string + wantValid bool + }{ + // Valid cases - only one field set + {"Only imageUrl", "quay.io/org/repo", "", "", nil, true}, + {"Only imageDigest", "", "sha256:cfe1335814d92eabecfe9802f13298539caa7bbd0a13b61f320dc45bdded473d", "", nil, true}, + {"Only imageRef", "", "", "sha256:cfe1335814d92eabecfe9802f13298539caa7bbd0a13b61f320dc45bdded473d", nil, true}, + {"Only componentNames", "", "", "", []string{"component1"}, true}, + {"No fields set", "", "", "", nil, true}, + + // Invalid cases - multiple fields set + {"imageUrl and imageDigest", "quay.io/org/repo", "sha256:cfe1335814d92eabecfe9802f13298539caa7bbd0a13b61f320dc45bdded473d", "", nil, false}, + {"imageUrl and imageRef", "quay.io/org/repo", "", "sha256:cfe1335814d92eabecfe9802f13298539caa7bbd0a13b61f320dc45bdded473d", nil, false}, + {"imageUrl and componentNames", "quay.io/org/repo", "", "", []string{"component1"}, false}, + {"imageDigest and imageRef", "", "sha256:cfe1335814d92eabecfe9802f13298539caa7bbd0a13b61f320dc45bdded473d", "sha256:1f88f9fb4543eadf97afcbd417c258fdf1a02dd000a36e39e7e4649d1b083b4e", nil, false}, + {"imageDigest and componentNames", "", "sha256:cfe1335814d92eabecfe9802f13298539caa7bbd0a13b61f320dc45bdded473d", "", []string{"component1"}, false}, + {"imageRef and componentNames", "", "", "sha256:cfe1335814d92eabecfe9802f13298539caa7bbd0a13b61f320dc45bdded473d", []string{"component1"}, false}, + {"Three fields set", "quay.io/org/repo", "sha256:cfe1335814d92eabecfe9802f13298539caa7bbd0a13b61f320dc45bdded473d", "", []string{"component1"}, false}, + {"All fields set", "quay.io/org/repo", "sha256:cfe1335814d92eabecfe9802f13298539caa7bbd0a13b61f320dc45bdded473d", "sha256:1f88f9fb4543eadf97afcbd417c258fdf1a02dd000a36e39e7e4649d1b083b4e", []string{"component1"}, false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Create a CRD validation schema + crd := v1.CustomResourceDefinition{} + bytes, err := os.ReadFile("../../config/crd/bases/appstudio.redhat.com_enterprisecontractpolicies.yaml") + if err != nil { + t.Fatalf("unexpected error reading CRD: %s", err) + } + if err := yaml.Unmarshal(bytes, &crd); err != nil { + t.Fatalf("unexpected error when decoding schema: %s", err) + } + + crdv := apiextensions.CustomResourceValidation{} + if err := v1.Convert_v1_CustomResourceValidation_To_apiextensions_CustomResourceValidation(crd.Spec.Versions[0].Schema, &crdv, nil); err != nil { + t.Fatalf("failed in CRD validation conversion: %s", err) + } + + s, err := schema.NewStructural(crdv.OpenAPIV3Schema) + if err != nil { + t.Fatalf("unexpected error when creating structural: %s", err) + } + + validator := validation.NewSchemaValidatorFromOpenAPI(s.ToKubeOpenAPI()) + + // Build the volatile criteria map + criteria := map[string]interface{}{ + "value": "test-rule", + } + if tt.imageUrl != "" { + criteria["imageUrl"] = tt.imageUrl + } + if tt.imageDigest != "" { + criteria["imageDigest"] = tt.imageDigest + } + if tt.imageRef != "" { + criteria["imageRef"] = tt.imageRef + } + if tt.componentNames != nil { + componentNamesInterface := make([]interface{}, len(tt.componentNames)) + for i, name := range tt.componentNames { + componentNamesInterface[i] = name + } + criteria["componentNames"] = componentNamesInterface + } + + // Convert policy to unstructured for validation + obj := unstructured.Unstructured{} + obj.SetUnstructuredContent(map[string]interface{}{ + "apiVersion": "appstudio.redhat.com/v1alpha1", + "kind": "EnterpriseContractPolicy", + "metadata": map[string]interface{}{ + "name": "test-policy", + }, + "spec": map[string]interface{}{ + "sources": []interface{}{ + map[string]interface{}{ + "volatileConfig": map[string]interface{}{ + "exclude": []interface{}{criteria}, + }, + }, + }, + }, + }) + + // Validate using custom resource strategy to include CEL validation + gvk := rts.GroupVersionKind{Group: "appstudio.redhat.com", Version: "v1alpha1", Kind: "EnterpriseContractPolicy"} + errs := customresource.NewStrategy(nil, false, gvk, validator, nil, s, nil, nil).Validate(context.Background(), &obj) + + isValid := len(errs) == 0 + if isValid != tt.wantValid { + t.Errorf("Validation = %v, want %v. Errors: %v", isValid, tt.wantValid, errs) + } + }) + } +} + +func TestComponentNamesField(t *testing.T) { + tests := []struct { + name string + componentNames []ComponentName + wantValid bool + omitField bool // true if the field should be omitted entirely + }{ + // Valid cases + {"Single component", []ComponentName{"component1"}, true, false}, + {"Multiple components", []ComponentName{"component1", "component2", "component3"}, true, false}, + {"Component with hyphens", []ComponentName{"my-component"}, true, false}, + {"Component with numbers", []ComponentName{"component123"}, true, false}, + {"Omitted field", nil, true, true}, // Field is omitted entirely + + // Valid edge case + {"Empty array", []ComponentName{}, true, false}, // Empty array is allowed + + // Invalid cases + {"Empty string", []ComponentName{""}, false, false}, // Violates MinLength=1 + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Create a policy with the test component names + policy := EnterpriseContractPolicy{ + Spec: EnterpriseContractPolicySpec{ + Sources: []Source{ + { + VolatileConfig: &VolatileSourceConfig{ + Exclude: []VolatileCriteria{ + { + Value: "test-rule", + }, + }, + }, + }, + }, + }, + } + if !tt.omitField { + policy.Spec.Sources[0].VolatileConfig.Exclude[0].ComponentNames = tt.componentNames + } + + // Create a CRD validation schema + crd := v1.CustomResourceDefinition{} + bytes, err := os.ReadFile("../../config/crd/bases/appstudio.redhat.com_enterprisecontractpolicies.yaml") + if err != nil { + t.Fatalf("unexpected error reading CRD: %s", err) + } + if err := yaml.Unmarshal(bytes, &crd); err != nil { + t.Fatalf("unexpected error when decoding schema: %s", err) + } + + crdv := apiextensions.CustomResourceValidation{} + if err := v1.Convert_v1_CustomResourceValidation_To_apiextensions_CustomResourceValidation(crd.Spec.Versions[0].Schema, &crdv, nil); err != nil { + t.Fatalf("failed in CRD validation conversion: %s", err) + } + + s, err := schema.NewStructural(crdv.OpenAPIV3Schema) + if err != nil { + t.Fatalf("unexpected error when creating structural: %s", err) + } + + v := validation.NewSchemaValidatorFromOpenAPI(s.ToKubeOpenAPI()) + + // Convert policy to unstructured for validation + obj := unstructured.Unstructured{} + obj.SetUnstructuredContent(map[string]interface{}{ + "apiVersion": "appstudio.redhat.com/v1alpha1", + "kind": "EnterpriseContractPolicy", + "spec": map[string]interface{}{ + "sources": []interface{}{ + map[string]interface{}{ + "volatileConfig": map[string]interface{}{ + "exclude": []interface{}{ + func() map[string]interface{} { + m := map[string]interface{}{ + "value": "test-rule", + } + if !tt.omitField { + // Convert []string to []interface{} + componentNamesInterface := make([]interface{}, len(tt.componentNames)) + for i, name := range tt.componentNames { + componentNamesInterface[i] = name + } + m["componentNames"] = componentNamesInterface + } + return m + }(), + }, + }, + }, + }, + }, + }) + + // Validate the object + result := v.Validate(&obj) + isValid := result.IsValid() + + if isValid != tt.wantValid { + t.Errorf("Validation for %v = %v, want %v. Errors: %v", tt.componentNames, isValid, tt.wantValid, result.Errors) + } + + // Also validate the actual policy object + // Note: Empty arrays with omitempty are omitted during JSON marshaling, + // so they're treated as omitted fields and pass validation + if tt.wantValid && !tt.omitField { + policyObj := unstructured.Unstructured{} + policyBytes, err := json.Marshal(policy) + if err != nil { + t.Fatalf("unexpected error marshaling policy: %s", err) + } + if err := json.Unmarshal(policyBytes, &policyObj.Object); err != nil { + t.Fatalf("unexpected error unmarshaling policy: %s", err) + } + + policyResult := v.Validate(&policyObj) + policyIsValid := policyResult.IsValid() + + if policyIsValid != tt.wantValid { + t.Errorf("Policy validation for %v = %v, want %v. Errors: %v", tt.componentNames, policyIsValid, tt.wantValid, policyResult.Errors) + } + } + }) + } +} diff --git a/api/v1alpha1/policy_spec.json b/api/v1alpha1/policy_spec.json index a2b8403..f65f111 100644 --- a/api/v1alpha1/policy_spec.json +++ b/api/v1alpha1/policy_spec.json @@ -180,6 +180,13 @@ "type": "string", "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,}$`" }, + "componentNames": { + "items": { + "type": "string" + }, + "type": "array", + "description": "ComponentNames is used to specify component names from\nApplicationSnapshot. This allows filtering in scenarios where\nmultiple components share the same image repository.\n+optional\n+listType=set" + }, "reference": { "type": "string", "description": "Reference is used to include a link to related information such as a Jira issue URL.\n+optional" diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index d8f1c33..c117a21 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -244,6 +244,11 @@ func (in *SourceConfig) DeepCopy() *SourceConfig { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *VolatileCriteria) DeepCopyInto(out *VolatileCriteria) { *out = *in + if in.ComponentNames != nil { + in, out := &in.ComponentNames, &out.ComponentNames + *out = make([]ComponentName, len(*in)) + copy(*out, *in) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new VolatileCriteria. @@ -262,12 +267,16 @@ func (in *VolatileSourceConfig) DeepCopyInto(out *VolatileSourceConfig) { if in.Exclude != nil { in, out := &in.Exclude, &out.Exclude *out = make([]VolatileCriteria, len(*in)) - copy(*out, *in) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } } if in.Include != nil { in, out := &in.Include, &out.Include *out = make([]VolatileCriteria, len(*in)) - copy(*out, *in) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } } } diff --git a/config/crd/bases/appstudio.redhat.com_enterprisecontractpolicies.yaml b/config/crd/bases/appstudio.redhat.com_enterprisecontractpolicies.yaml index ccdfef3..60d1065 100644 --- a/config/crd/bases/appstudio.redhat.com_enterprisecontractpolicies.yaml +++ b/config/crd/bases/appstudio.redhat.com_enterprisecontractpolicies.yaml @@ -159,6 +159,16 @@ spec: items: description: VolatileCriteria includes or excludes a policy rule with effective dates as an option. properties: + componentNames: + description: |- + ComponentNames is used to specify component names from + ApplicationSnapshot. This allows filtering in scenarios where + multiple components share the same image repository. + items: + minLength: 1 + type: string + type: array + x-kubernetes-list-type: set effectiveOn: format: date-time type: string @@ -187,6 +197,9 @@ spec: required: - value type: object + x-kubernetes-validations: + - message: only one of imageUrl, imageDigest, imageRef, or componentNames may be set + rule: '(has(self.imageUrl) ? 1 : 0) + (has(self.imageDigest) ? 1 : 0) + (has(self.imageRef) ? 1 : 0) + (has(self.componentNames) ? 1 : 0) <= 1' type: array include: description: |- @@ -195,6 +208,16 @@ spec: items: description: VolatileCriteria includes or excludes a policy rule with effective dates as an option. properties: + componentNames: + description: |- + ComponentNames is used to specify component names from + ApplicationSnapshot. This allows filtering in scenarios where + multiple components share the same image repository. + items: + minLength: 1 + type: string + type: array + x-kubernetes-list-type: set effectiveOn: format: date-time type: string @@ -223,6 +246,9 @@ spec: required: - value type: object + x-kubernetes-validations: + - message: only one of imageUrl, imageDigest, imageRef, or componentNames may be set + rule: '(has(self.imageUrl) ? 1 : 0) + (has(self.imageDigest) ? 1 : 0) + (has(self.imageRef) ? 1 : 0) + (has(self.componentNames) ? 1 : 0) <= 1' type: array type: object type: object