From 9bafbbfdcef9eb4a2ac3cac19bccc47a30513c29 Mon Sep 17 00:00:00 2001 From: Rachimypala-RM Date: Wed, 17 Jun 2026 23:04:34 +0530 Subject: [PATCH 1/3] apis: restrict secretRef to tls component only (PROJQUAY-11867) secretRef was exposed on the generic Component struct without any kind-level guard, allowing it to be set on components such as postgres, redis, and clair where it has no effect. The only consumer of SecretRef in the codebase is GetTLSSecretRef, which already gates on kind == tls. Add a supportsSecretRef allowlist (tls only), a new CEL admission rule that rejects secretRef on any component whose kind is not tls, and a ValidateOverrides runtime check that fires during reconciliation as a belt-and-suspenders guard. Co-Authored-By: Claude Sonnet 4.6 --- apis/quay/v1/quayregistry_types.go | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/apis/quay/v1/quayregistry_types.go b/apis/quay/v1/quayregistry_types.go index b9b0e99e7..026edb7e5 100644 --- a/apis/quay/v1/quayregistry_types.go +++ b/apis/quay/v1/quayregistry_types.go @@ -133,6 +133,10 @@ var supportsTLSOverride = []ComponentKind{ ComponentClairPostgres, } +var supportsSecretRef = []ComponentKind{ + ComponentTLS, +} + const ( ManagedKeysName = "quay-registry-managed-secret-keys" QuayConfigTLSSecretName = "quay-config-tls" @@ -160,6 +164,7 @@ type QuayRegistrySpec struct { // +kubebuilder:validation:XValidation:rule="!has(self.secretRef) || self.secretRef.name.size() != 0",message="secretRef.name must not be empty" // +kubebuilder:validation:XValidation:rule="!has(self.overrides) || !has(self.overrides.tls) || !has(self.overrides.tls.secretRef) || self.overrides.tls.enabled",message="tls.secretRef requires tls.enabled to be true" // +kubebuilder:validation:XValidation:rule="!has(self.overrides) || !has(self.overrides.tls) || !has(self.overrides.tls.secretRef) || self.overrides.tls.secretRef.name.size() != 0",message="tls.secretRef.name must not be empty" +// +kubebuilder:validation:XValidation:rule="!has(self.secretRef) || self.kind == 'tls'",message="secretRef is only supported for the tls component" type Component struct { // Kind is the unique name of this type of component. Kind ComponentKind `json:"kind"` @@ -560,6 +565,14 @@ func hasAffinity(component Component) bool { func ValidateOverrides(quay *QuayRegistry) error { for _, component := range quay.Spec.Components { + // secretRef is independent of Overrides; check it for every component. + if component.SecretRef != nil && !ComponentSupportsOverride(component.Kind, "secretRef") { + return fmt.Errorf( + "component %s does not support secretRef", + component.Kind, + ) + } + // No overrides provided if component.Overrides == nil { continue @@ -838,6 +851,8 @@ func ComponentSupportsOverride(component ComponentKind, override string) bool { components = supportsSecurityContextOverride case "tls": components = supportsTLSOverride + case "secretRef": + components = supportsSecretRef } for _, cmp := range components { From 984e9ddacac61ae8834d871bd15d63273a260651 Mon Sep 17 00:00:00 2001 From: Rachimypala-RM Date: Wed, 17 Jun 2026 23:15:01 +0530 Subject: [PATCH 2/3] apis: regenerate CRD with secretRef tls-only CEL rule (PROJQUAY-11867) The XValidation marker added to Component in quayregistry_types.go was not reflected in the generated CRD manifests. Manually patched both bundle/manifests/quayregistries.crd.yaml and config/crd/bases/quay.redhat.com_quayregistries.yaml to include the new x-kubernetes-validations entry that rejects secretRef on any component kind other than tls, matching the source marker exactly. Co-Authored-By: Claude Sonnet 4.6 --- bundle/manifests/quayregistries.crd.yaml | 2 ++ config/crd/bases/quay.redhat.com_quayregistries.yaml | 2 ++ 2 files changed, 4 insertions(+) diff --git a/bundle/manifests/quayregistries.crd.yaml b/bundle/manifests/quayregistries.crd.yaml index 35a438ae2..d27405f16 100644 --- a/bundle/manifests/quayregistries.crd.yaml +++ b/bundle/manifests/quayregistries.crd.yaml @@ -1238,6 +1238,8 @@ spec: - message: tls.secretRef.name must not be empty rule: '!has(self.overrides) || !has(self.overrides.tls) || !has(self.overrides.tls.secretRef) || self.overrides.tls.secretRef.name.size() != 0' + - message: secretRef is only supported for the tls component + rule: '!has(self.secretRef) || self.kind == ''tls''' type: array configBundleSecret: description: |- diff --git a/config/crd/bases/quay.redhat.com_quayregistries.yaml b/config/crd/bases/quay.redhat.com_quayregistries.yaml index 35a438ae2..d27405f16 100644 --- a/config/crd/bases/quay.redhat.com_quayregistries.yaml +++ b/config/crd/bases/quay.redhat.com_quayregistries.yaml @@ -1238,6 +1238,8 @@ spec: - message: tls.secretRef.name must not be empty rule: '!has(self.overrides) || !has(self.overrides.tls) || !has(self.overrides.tls.secretRef) || self.overrides.tls.secretRef.name.size() != 0' + - message: secretRef is only supported for the tls component + rule: '!has(self.secretRef) || self.kind == ''tls''' type: array configBundleSecret: description: |- From d31058cd9e4935dfdf8d0d8745312ab3be44c9a3 Mon Sep 17 00:00:00 2001 From: Brady Pratt Date: Wed, 24 Jun 2026 02:54:35 -0500 Subject: [PATCH 3/3] PROJQUAY-11867: test(apis): add unit tests for secretRef tls-only restriction Co-Authored-By: Claude Opus 4.6 (1M context) --- apis/quay/v1/quayregistry_types_test.go | 78 +++++++++++++++++++++++++ 1 file changed, 78 insertions(+) diff --git a/apis/quay/v1/quayregistry_types_test.go b/apis/quay/v1/quayregistry_types_test.go index 32527bef0..edbdd5de9 100644 --- a/apis/quay/v1/quayregistry_types_test.go +++ b/apis/quay/v1/quayregistry_types_test.go @@ -755,6 +755,63 @@ var validateOverridesTests = []struct { }, nil, }, + { + "SecretRefOnTLSComponentUnmanaged", + QuayRegistry{ + Spec: QuayRegistrySpec{ + Components: []Component{ + {Kind: "tls", Managed: false, SecretRef: &corev1.LocalObjectReference{Name: "my-tls-secret"}}, + }, + }, + }, + nil, + }, + { + "SecretRefOnPostgresRejected", + QuayRegistry{ + Spec: QuayRegistrySpec{ + Components: []Component{ + {Kind: "postgres", Managed: false, SecretRef: &corev1.LocalObjectReference{Name: "pg-secret"}}, + }, + }, + }, + errors.New("component postgres does not support secretRef"), + }, + { + "SecretRefOnRedisRejected", + QuayRegistry{ + Spec: QuayRegistrySpec{ + Components: []Component{ + {Kind: "redis", Managed: false, SecretRef: &corev1.LocalObjectReference{Name: "redis-secret"}}, + }, + }, + }, + errors.New("component redis does not support secretRef"), + }, + { + "SecretRefOnClairRejected", + QuayRegistry{ + Spec: QuayRegistrySpec{ + Components: []Component{ + {Kind: "clair", Managed: false, SecretRef: &corev1.LocalObjectReference{Name: "clair-secret"}}, + }, + }, + }, + errors.New("component clair does not support secretRef"), + }, + { + "NoSecretRefNoError", + QuayRegistry{ + Spec: QuayRegistrySpec{ + Components: []Component{ + {Kind: "postgres", Managed: true}, + {Kind: "redis", Managed: true}, + {Kind: "tls", Managed: true}, + }, + }, + }, + nil, + }, } func TestValidOverrides(t *testing.T) { @@ -1044,6 +1101,27 @@ func TestGetTLSOverrideForComponent(t *testing.T) { } } +func TestComponentSupportsSecretRefOverride(t *testing.T) { + tests := []struct { + kind ComponentKind + expected bool + }{ + {ComponentTLS, true}, + {ComponentPostgres, false}, + {ComponentRedis, false}, + {ComponentClair, false}, + {ComponentQuay, false}, + {ComponentMirror, false}, + {ComponentObjectStorage, false}, + } + + for _, tt := range tests { + t.Run(string(tt.kind), func(t *testing.T) { + assert.Equal(t, tt.expected, ComponentSupportsOverride(tt.kind, "secretRef")) + }) + } +} + func resourcePtr(s string) *resource.Quantity { q := resource.MustParse(s) return &q