httpSecret should be generated when it is not set

openshift · Sep 10, 2020 · 1f6c44a · 1f6c44a
1 parent e528a06
commit 1f6c44a
Show file tree

Hide file tree

Showing 5 changed files with 69 additions and 16 deletions.
diff --git a/pkg/operator/bootstrap.go b/pkg/operator/bootstrap.go
@@ -2,7 +2,6 @@ package operator
 
 import (
 	"context"
-	"crypto/rand"
 	"fmt"
 
 	appsapi "k8s.io/api/apps/v1"
@@ -22,10 +21,6 @@ import (
 	"github.com/openshift/cluster-image-registry-operator/pkg/storage/util"
 )
 
-// randomSecretSize is the number of random bytes to generate
-// for the http secret
-const randomSecretSize = 64
-
 // Bootstrap registers this operator with OpenShift by creating an appropriate
 // ClusterOperator custom resource. This function also creates the initial
 // configuration for the Image Registry.
@@ -43,11 +38,6 @@ func (c *Controller) Bootstrap() error {
 	// If no registry resource exists, let's create one with sane defaults
 	klog.Infof("generating registry custom resource")
 
-	var secretBytes [randomSecretSize]byte
-	if _, err := rand.Read(secretBytes[:]); err != nil {
-		return fmt.Errorf("could not generate random bytes for HTTP secret: %s", err)
-	}
-
 	platformStorage, replicas, err := storage.GetPlatformStorage(c.listers)
 	if err != nil {
 		return err
@@ -85,7 +75,6 @@ func (c *Controller) Bootstrap() error {
 			ManagementState: mgmtState,
 			Storage:         platformStorage,
 			Replicas:        replicas,
-			HTTPSecret:      fmt.Sprintf("%x", string(secretBytes[:])),
 			RolloutStrategy: string(rolloutStrategy),
 		},
 		Status: imageregistryv1.ImageRegistryStatus{},

diff --git a/pkg/operator/bootstrap_test.go b/pkg/operator/bootstrap_test.go
@@ -68,11 +68,6 @@ func TestBootstrapAWS(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	if config.Spec.HTTPSecret == "" {
-		t.Errorf("got empty spec.httpSecrets, want random string")
-	}
-	config.Spec.HTTPSecret = ""
-
 	expected := imageregistryv1.ImageRegistrySpec{
 		ManagementState: "Managed",
 		Storage: imageregistryv1.ImageRegistryConfigStorage{

diff --git a/pkg/operator/complete.go b/pkg/operator/complete.go
@@ -1,13 +1,18 @@
 package operator
 
 import (
+	"crypto/rand"
 	"fmt"
 
 	imageregistryv1 "github.com/openshift/api/imageregistry/v1"
 
 	"github.com/openshift/cluster-image-registry-operator/pkg/defaults"
 )
 
+// randomSecretSize is the number of random bytes to generate
+// for the http secret
+const randomSecretSize = 64
+
 func appendFinalizer(cr *imageregistryv1.Config) {
 	for i := range cr.ObjectMeta.Finalizers {
 		if cr.ObjectMeta.Finalizers[i] == defaults.ImageRegistryOperatorResourceFinalizer {
@@ -37,3 +42,16 @@ func verifyResource(cr *imageregistryv1.Config) error {
 
 	return nil
 }
+
+func applyDefaults(cr *imageregistryv1.Config) error {
+	if cr.Spec.HTTPSecret == "" {
+		var secretBytes [randomSecretSize]byte
+		if _, err := rand.Read(secretBytes[:]); err != nil {
+			return fmt.Errorf("could not generate random bytes for HTTP secret: %s", err)
+		}
+
+		cr.Spec.HTTPSecret = fmt.Sprintf("%x", string(secretBytes[:]))
+	}
+
+	return nil
+}
diff --git a/pkg/operator/controller.go b/pkg/operator/controller.go
@@ -189,6 +189,11 @@ func (c *Controller) createOrUpdateResources(cr *imageregistryv1.Config) error {
 		return newPermanentError("VerificationFailed", fmt.Errorf("unable to complete resource: %s", err))
 	}
 
+	err = applyDefaults(cr)
+	if err != nil {
+		return err
+	}
+
 	err = c.generator.Apply(cr)
 	if err == storage.ErrStorageNotConfigured {
 		return newPermanentError("StorageNotConfigured", err)

diff --git a/test/e2e/configuration_test.go b/test/e2e/configuration_test.go
@@ -29,6 +29,52 @@ import (
 	"github.com/openshift/cluster-image-registry-operator/test/framework"
 )
 
+func TestHTTPSecretDefaulter(t *testing.T) {
+	te := framework.SetupAvailableImageRegistry(t, &imageregistryapiv1.ImageRegistrySpec{
+		ManagementState: operatorapiv1.Managed,
+		Storage: imageregistryapiv1.ImageRegistryConfigStorage{
+			EmptyDir: &imageregistryapiv1.ImageRegistryConfigStorageEmptyDir{},
+		},
+		Replicas: 1,
+	})
+	defer framework.TeardownImageRegistry(te)
+
+	cr, err := te.Client().Configs().Get(context.Background(), defaults.ImageRegistryResourceName, metav1.GetOptions{})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if cr.Spec.HTTPSecret == "" {
+		t.Errorf("got empty spec.httpSecrets, want random string")
+	}
+
+	firstSecret := cr.Spec.HTTPSecret
+
+	if _, err := te.Client().Configs().Patch(
+		context.Background(),
+		defaults.ImageRegistryResourceName,
+		types.JSONPatchType,
+		framework.MarshalJSON([]framework.JSONPatch{
+			{
+				Op:    "replace",
+				Path:  "/spec/httpSecret",
+				Value: "",
+			},
+		}),
+		metav1.PatchOptions{},
+	); err != nil {
+		t.Fatalf("unable to reset httpSecret: %s", err)
+	}
+
+	cr = framework.WaitUntilImageRegistryConfigIsProcessed(te)
+	if cr.Spec.HTTPSecret == "" {
+		t.Errorf("got empty spec.httpSecrets, want it to be regenerated")
+	}
+	if cr.Spec.HTTPSecret == firstSecret {
+		t.Errorf("regenerated spec.httpSecrets %q is the same, want it to be randomized", cr.Spec.HTTPSecret)
+	}
+}
+
 func TestPodResourceConfiguration(t *testing.T) {
 	te := framework.SetupAvailableImageRegistry(t, &imageregistryapiv1.ImageRegistrySpec{
 		ManagementState: operatorapiv1.Managed,