env: turning 'base' a component (PROJQUAY-3055)

To avoid having an Override reference directly into QuayRegitry's Spec we need to turn "base" into a component. This PR also adds e2e tests for the env var override feature.
quay · Feb 23, 2022 · 6a0b71c · 6a0b71c
1 parent 351a29d
commit 6a0b71c
Show file tree

Hide file tree

Showing 16 changed files with 289 additions and 322 deletions.
diff --git a/apis/quay/v1/quayregistry_types.go b/apis/quay/v1/quayregistry_types.go
@@ -17,7 +17,6 @@ limitations under the License.
 package v1
 
 import (
-	"errors"
 	"fmt"
 	"os"
 	"strings"
@@ -52,6 +51,7 @@ const (
 )
 
 var allComponents = []ComponentKind{
+	ComponentBase,
 	ComponentPostgres,
 	ComponentClair,
 	ComponentRedis,
@@ -78,6 +78,7 @@ var supportsVolumeOverride = []ComponentKind{
 }
 
 var supportsEnvOverride = []ComponentKind{
+	ComponentBase,
 	ComponentClair,
 	ComponentMirror,
 	ComponentPostgres,
@@ -97,8 +98,6 @@ type QuayRegistrySpec struct {
 	ConfigBundleSecret string `json:"configBundleSecret,omitempty"`
 	// Components declare how the Operator should handle backing Quay services.
 	Components []Component `json:"components,omitempty"`
-	// Overrides holds overrides for the Base component (quay-app).
-	Overrides *Override `json:"overrides,omitempty"`
 }
 
 // Component describes how the Operator should handle a backing Quay service.
@@ -337,58 +336,94 @@ func RequiredComponent(component ComponentKind) bool {
 }
 
 // EnsureDefaultComponents adds any `Components` which are missing from `Spec.Components`.
-// Returns an error if a component was declared as managed but is not supported in the current k8s cluster.
-func EnsureDefaultComponents(ctx *quaycontext.QuayRegistryContext, quay *QuayRegistry) (*QuayRegistry, error) {
-	updatedQuay := quay.DeepCopy()
-	if updatedQuay.Spec.Components == nil {
-		updatedQuay.Spec.Components = []Component{}
+// Returns an error if a component was declared as managed but is not supported in the current
+// k8s cluster.
+func EnsureDefaultComponents(ctx *quaycontext.QuayRegistryContext, quay *QuayRegistry) error {
+	if quay.Spec.Components == nil {
+		quay.Spec.Components = []Component{}
 	}
 
-	type componentCheck struct {
+	type check struct {
 		check func() bool
 		msg   string
 	}
-	componentChecks := map[ComponentKind]componentCheck{
-		ComponentRoute:         {func() bool { return ctx.SupportsRoutes }, "cannot use `route` component when `Route` API not available"},
-		ComponentTLS:           {func() bool { return ctx.SupportsRoutes }, "cannot use `tls` component when `Route` API not available"},
-		ComponentObjectStorage: {func() bool { return ctx.SupportsObjectStorage }, "cannot use `ObjectStorage` component when `ObjectStorage` API not available"},
-		ComponentMonitoring:    {func() bool { return ctx.SupportsMonitoring }, "cannot use `monitoring` component when `Prometheus` API not available"},
+	checks := map[ComponentKind]check{
+		ComponentRoute: {
+			check: func() bool { return ctx.SupportsRoutes },
+			msg:   "Route API not available",
+		},
+		ComponentTLS: {
+			check: func() bool { return ctx.SupportsRoutes },
+			msg:   "Route API not available",
+		},
+		ComponentObjectStorage: {
+			check: func() bool { return ctx.SupportsObjectStorage },
+			msg:   "ObjectStorage API not available",
+		},
+		ComponentMonitoring: {
+			check: func() bool { return ctx.SupportsMonitoring },
+			msg:   "Prometheus API not available",
+		},
 	}
 
-	componentManaged := map[ComponentKind]componentCheck{
+	componentManaged := map[ComponentKind]check{
 		ComponentTLS: {
 			check: func() bool { return ctx.TLSCert == nil && ctx.TLSKey == nil },
 		},
 	}
 
-	for _, component := range allComponents {
-		componentCheck, checkExists := componentChecks[component]
-		if (checkExists && !componentCheck.check()) && ComponentIsManaged(quay.Spec.Components, component) {
-			return quay, errors.New(componentCheck.msg)
+	for _, cmp := range allComponents {
+		ccheck, checkexists := checks[cmp]
+		if checkexists {
+			// if there is a check registered for the component we run it, if the
+			// check fails and the component is managed then we have a problem with
+			// the current components configuration. returns the check error.
+			if !ccheck.check() && ComponentIsManaged(quay.Spec.Components, cmp) {
+				return fmt.Errorf(
+					"Error validatingcomponent %s: %s", cmp, ccheck.msg,
+				)
+			}
 		}
 
-		found := false
-		for _, declaredComponent := range quay.Spec.Components {
-			if component == declaredComponent.Kind {
-				found = true
-				break
+		// if the component has already been declared in the QuayRegistry object we can
+		// just return as there is nothing we need to do.
+		var found bool
+		for i, declaredComponent := range quay.Spec.Components {
+			if cmp != declaredComponent.Kind {
+				continue
+			}
+
+			// we disregard whatever the user has defined for base component, this
+			// is a component that can't be unmanaged so if user sets it to unmanaged
+			// we are going to roll it back to managed.
+			if declaredComponent.Kind == ComponentBase {
+				quay.Spec.Components[i].Managed = true
 			}
+
+			found = true
+			break
+		}
+		if found {
+			continue
 		}
 
-		managed := !checkExists || componentCheck.check()
-		if _, ok := componentManaged[component]; ok {
-			managed = managed && componentManaged[component].check()
+		// the component management status is set to true if the check for the component
+		// has passed.
+		managed := !checkexists || ccheck.check()
+		if _, ok := componentManaged[cmp]; ok {
+			managed = managed && componentManaged[cmp].check()
 		}
 
-		if !found {
-			updatedQuay.Spec.Components = append(updatedQuay.Spec.Components, Component{
-				Kind:    component,
+		quay.Spec.Components = append(
+			quay.Spec.Components,
+			Component{
+				Kind:    cmp,
 				Managed: managed,
-			})
-		}
+			},
+		)
 	}
 
-	return updatedQuay, nil
+	return nil
 }
 
 // ValidateOverrides validates that the overrides set for each component are valid.
@@ -580,6 +615,8 @@ func FieldGroupNameFor(cmp ComponentKind) (string, error) {
 		return "", nil
 	case ComponentTLS:
 		return "", nil
+	case ComponentBase:
+		return "", nil
 	default:
 		return "", fmt.Errorf("unknown component: %q", cmp)
 	}

diff --git a/apis/quay/v1/quayregistry_types_test.go b/apis/quay/v1/quayregistry_types_test.go
@@ -23,6 +23,7 @@ var ensureDefaultComponentsTests = []struct {
 		QuayRegistry{
 			Spec: QuayRegistrySpec{
 				Components: []Component{
+					{Kind: "base", Managed: true},
 					{Kind: "postgres", Managed: true},
 					{Kind: "redis", Managed: true},
 					{Kind: "clair", Managed: true},
@@ -42,6 +43,7 @@ var ensureDefaultComponentsTests = []struct {
 			SupportsRoutes:        true,
 		},
 		[]Component{
+			{Kind: "base", Managed: true},
 			{Kind: "postgres", Managed: true},
 			{Kind: "redis", Managed: true},
 			{Kind: "clair", Managed: true},
@@ -60,6 +62,7 @@ var ensureDefaultComponentsTests = []struct {
 		QuayRegistry{
 			Spec: QuayRegistrySpec{
 				Components: []Component{
+					{Kind: "base", Managed: true},
 					{Kind: "postgres", Managed: true},
 					{Kind: "redis", Managed: true},
 					{Kind: "clair", Managed: true},
@@ -74,6 +77,7 @@ var ensureDefaultComponentsTests = []struct {
 		},
 		quaycontext.QuayRegistryContext{},
 		[]Component{
+			{Kind: "base", Managed: true},
 			{Kind: "postgres", Managed: true},
 			{Kind: "redis", Managed: true},
 			{Kind: "clair", Managed: true},
@@ -91,6 +95,7 @@ var ensureDefaultComponentsTests = []struct {
 		QuayRegistry{
 			Spec: QuayRegistrySpec{
 				Components: []Component{
+					{Kind: "base", Managed: true},
 					{Kind: "postgres", Managed: true},
 					{Kind: "redis", Managed: true},
 					{Kind: "clair", Managed: true},
@@ -110,6 +115,7 @@ var ensureDefaultComponentsTests = []struct {
 			SupportsMonitoring:    true,
 		},
 		[]Component{
+			{Kind: "base", Managed: true},
 			{Kind: "postgres", Managed: true},
 			{Kind: "redis", Managed: true},
 			{Kind: "clair", Managed: true},
@@ -128,6 +134,7 @@ var ensureDefaultComponentsTests = []struct {
 		QuayRegistry{
 			Spec: QuayRegistrySpec{
 				Components: []Component{
+					{Kind: "base", Managed: true},
 					{Kind: "postgres", Managed: true},
 					{Kind: "redis", Managed: true},
 					{Kind: "clair", Managed: true},
@@ -147,6 +154,7 @@ var ensureDefaultComponentsTests = []struct {
 			SupportsMonitoring:    true,
 		},
 		[]Component{
+			{Kind: "base", Managed: true},
 			{Kind: "postgres", Managed: true},
 			{Kind: "redis", Managed: true},
 			{Kind: "clair", Managed: true},
@@ -167,6 +175,7 @@ var ensureDefaultComponentsTests = []struct {
 		},
 		quaycontext.QuayRegistryContext{},
 		[]Component{
+			{Kind: "base", Managed: true},
 			{Kind: "postgres", Managed: true},
 			{Kind: "redis", Managed: true},
 			{Kind: "clair", Managed: true},
@@ -191,6 +200,7 @@ var ensureDefaultComponentsTests = []struct {
 			SupportsMonitoring: true,
 		},
 		[]Component{
+			{Kind: "base", Managed: true},
 			{Kind: "postgres", Managed: true},
 			{Kind: "redis", Managed: true},
 			{Kind: "clair", Managed: true},
@@ -217,6 +227,7 @@ var ensureDefaultComponentsTests = []struct {
 		},
 		quaycontext.QuayRegistryContext{},
 		[]Component{
+			{Kind: "base", Managed: true},
 			{Kind: "postgres", Managed: false},
 			{Kind: "redis", Managed: true},
 			{Kind: "clair", Managed: true},
@@ -247,6 +258,7 @@ var ensureDefaultComponentsTests = []struct {
 			ClusterHostname:    "apps.example.com",
 		},
 		[]Component{
+			{Kind: "base", Managed: true},
 			{Kind: "postgres", Managed: false},
 			{Kind: "redis", Managed: true},
 			{Kind: "clair", Managed: true},
@@ -279,6 +291,7 @@ var ensureDefaultComponentsTests = []struct {
 			ClusterHostname:    "apps.example.com",
 		},
 		[]Component{
+			{Kind: "base", Managed: true},
 			{Kind: "postgres", Managed: false},
 			{Kind: "redis", Managed: true},
 			{Kind: "clair", Managed: true},
@@ -298,7 +311,8 @@ func TestEnsureDefaultComponents(t *testing.T) {
 	assert := assert.New(t)
 
 	for _, test := range ensureDefaultComponentsTests {
-		updatedQuay, err := EnsureDefaultComponents(&test.ctx, &test.quay)
+		updatedQuay := &test.quay
+		err := EnsureDefaultComponents(&test.ctx, &test.quay)
 
 		if test.expectedErr != nil {
 			assert.NotNil(err, test.name)

diff --git a/apis/quay/v1/zz_generated.deepcopy.go b/apis/quay/v1/zz_generated.deepcopy.go
diff --git a/bundle/manifests/quayregistries.crd.yaml b/bundle/manifests/quayregistries.crd.yaml
@@ -140,95 +140,6 @@ spec:
               configBundleSecret:
                 description: ConfigBundleSecret is the name of the Kubernetes `Secret` in the same namespace which contains the base Quay config and extra certs.
                 type: string
-              overrides:
-                description: Overrides holds overrides for the Base component (quay-app).
-                properties:
-                  env:
-                    items:
-                      description: EnvVar represents an environment variable present in a Container.
-                      properties:
-                        name:
-                          description: Name of the environment variable. Must be a C_IDENTIFIER.
-                          type: string
-                        value:
-                          description: 'Variable references $(VAR_NAME) are expanded using the previously defined environment variables in the container and any service environment variables. If a variable cannot be resolved, the reference in the input string will be unchanged. Double $$ are reduced to a single $, which allows for escaping the $(VAR_NAME) syntax: i.e. "$$(VAR_NAME)" will produce the string literal "$(VAR_NAME)". Escaped references will never be expanded, regardless of whether the variable exists or not. Defaults to "".'
-                          type: string
-                        valueFrom:
-                          description: Source for the environment variable's value. Cannot be used if value is not empty.
-                          properties:
-                            configMapKeyRef:
-                              description: Selects a key of a ConfigMap.
-                              properties:
-                                key:
-                                  description: The key to select.
-                                  type: string
-                                name:
-                                  description: 'Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names TODO: Add other useful fields. apiVersion, kind, uid?'
-                                  type: string
-                                optional:
-                                  description: Specify whether the ConfigMap or its key must be defined
-                                  type: boolean
-                              required:
-                              - key
-                              type: object
-                            fieldRef:
-                              description: 'Selects a field of the pod: supports metadata.name, metadata.namespace, `metadata.labels[''<KEY>'']`, `metadata.annotations[''<KEY>'']`, spec.nodeName, spec.serviceAccountName, status.hostIP, status.podIP, status.podIPs.'
-                              properties:
-                                apiVersion:
-                                  description: Version of the schema the FieldPath is written in terms of, defaults to "v1".
-                                  type: string
-                                fieldPath:
-                                  description: Path of the field to select in the specified API version.
-                                  type: string
-                              required:
-                              - fieldPath
-                              type: object
-                            resourceFieldRef:
-                              description: 'Selects a resource of the container: only resources limits and requests (limits.cpu, limits.memory, limits.ephemeral-storage, requests.cpu, requests.memory and requests.ephemeral-storage) are currently supported.'
-                              properties:
-                                containerName:
-                                  description: 'Container name: required for volumes, optional for env vars'
-                                  type: string
-                                divisor:
-                                  anyOf:
-                                  - type: integer
-                                  - type: string
-                                  description: Specifies the output format of the exposed resources, defaults to "1"
-                                  pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$
-                                  x-kubernetes-int-or-string: true
-                                resource:
-                                  description: 'Required: resource to select'
-                                  type: string
-                              required:
-                              - resource
-                              type: object
-                            secretKeyRef:
-                              description: Selects a key of a secret in the pod's namespace
-                              properties:
-                                key:
-                                  description: The key of the secret to select from.  Must be a valid secret key.
-                                  type: string
-                                name:
-                                  description: 'Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names TODO: Add other useful fields. apiVersion, kind, uid?'
-                                  type: string
-                                optional:
-                                  description: Specify whether the Secret or its key must be defined
-                                  type: boolean
-                              required:
-                              - key
-                              type: object
-                          type: object
-                      required:
-                      - name
-                      type: object
-                    type: array
-                  volumeSize:
-                    anyOf:
-                    - type: integer
-                    - type: string
-                    pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$
-                    x-kubernetes-int-or-string: true
-                type: object
             type: object
           status:
             description: QuayRegistryStatus defines the observed state of QuayRegistry.