Fixup #1

pkg/operator/boundsatokensignercontroller/controller.go

@@ -35,11 +35,14 @@ const (
 	operatorNamespace = operatorclient.OperatorNamespace
 	targetNamespace   = operatorclient.TargetNamespace
 
-	keySize                  = 2048
+	keySize = 2048
+	// A new keypair will first be written to this secret in the operator namespace...
 	NextSigningKeySecretName = "next-bound-service-account-signing-key"
-	SigningKeySecretName     = "bound-service-account-signing-key"
-	PrivateKeyKey            = "service-account.key"
-	PublicKeyKey             = "service-account.pub"
+	// ...and will copied to this secret in the operand namespace once
+	// it is safe to do so (i.e. public key present on master nodes).
+	SigningKeySecretName = "bound-service-account-signing-key"
+	PrivateKeyKey        = "service-account.key"
+	PublicKeyKey         = "service-account.pub"
 
 	PublicKeyConfigMapName = "bound-sa-token-signing-certs"
 )
@@ -89,26 +92,26 @@ func NewBoundSATokenSignerController(
 }
 
 func (c *BoundSATokenSignerController) sync() bool {
-	syncFailed := false
+	success := true
 	syncMethods := []func() error{
-		c.ensureOperatorSigningSecret,
+		c.ensureNextOperatorSigningSecret,
 		c.ensurePublicKeyConfigMap,
 		c.ensureOperandSigningSecret,
 	}
 	for _, syncMethod := range syncMethods {
 		err := syncMethod()
 		if err != nil {
 			utilruntime.HandleError(err)
-			syncFailed = true
+			success = false
 		}
 	}
-	return syncFailed
+	return success
 }
 
-// ensureOperatorSigningSecret ensures the existence of a secret in the operator
+// ensureNextOperatorSigningSecret ensures the existence of a secret in the operator
 // namespace containing an RSA keypair used for signing and validating bound service
 // account tokens.
-func (c *BoundSATokenSignerController) ensureOperatorSigningSecret() error {
+func (c *BoundSATokenSignerController) ensureNextOperatorSigningSecret() error {
 	// Attempt to retrieve the operator secret
 	secret, err := c.secretClient.Secrets(operatorNamespace).Get(NextSigningKeySecretName, metav1.GetOptions{})
 	if err != nil && !errors.IsNotFound(err) {
@@ -119,7 +122,7 @@ func (c *BoundSATokenSignerController) ensureOperatorSigningSecret() error {
 	needKeypair := secret == nil || len(secret.Data[PrivateKeyKey]) == 0 || len(secret.Data[PublicKeyKey]) == 0
 	if needKeypair {
 		klog.V(2).Infof("Creating a new signing secret for bound service account tokens.")
-		newSecret, err := newSigningSecret()
+		newSecret, err := newNextSigningSecret()
 		if err != nil {
 			return err
 		}
@@ -157,12 +160,14 @@ func (c *BoundSATokenSignerController) ensurePublicKeyConfigMap() error {
 				Namespace: targetNamespace,
 				Name:      PublicKeyConfigMapName,
 			},
-			Data: map[string]string{},
 		}
 	} else {
 		// Make a copy to avoid mutating the cache
 		configMap = cachedConfigMap.DeepCopy()
 	}
+	if configMap.Data == nil {
+		configMap.Data = map[string]string{}
+	}
 
 	currPublicKey := string(operatorSecret.Data[PublicKeyKey])
 	hasKey := configMapHasValue(configMap, currPublicKey)
@@ -174,7 +179,7 @@ func (c *BoundSATokenSignerController) ensurePublicKeyConfigMap() error {
 		// minimize the potential for not being able to validate issued tokens.
 		nextKeyIndex := len(configMap.Data) + 1
 		nextKeyKey := ""
-		for len(nextKeyKey) == 0 {
+		for {
 			possibleKey := fmt.Sprintf("service-account-%03d.pub", nextKeyIndex)
 			_, ok := configMap.Data[possibleKey]
 			if !ok {
@@ -234,27 +239,27 @@ func (c *BoundSATokenSignerController) ensureOperandSigningSecret() error {
 		return fmt.Errorf("unable to promote bound sa token signing key until public key configmap has been updated")
 	}
 
-	syncRequired := false
+	syncAllowed := false
 
 	if operandSecret == nil {
 		// If the operand secret is missing, it must be created to ensure the
 		// installer can proceed regardless of whether public keys have already been
 		// synced to the master nodes.
-		syncRequired = true
+		syncAllowed = true
 	} else {
 		// Update the operand secret only if the current public key has been synced to
 		// all nodes.
-		syncRequired, err = c.publicKeySyncedToAllNodes(currPublicKey)
+		syncAllowed, err = c.publicKeySyncedToAllNodes(currPublicKey)
 		if err != nil {
 			return err
 		}
-		if syncRequired {
+		if syncAllowed {
 			klog.V(2).Info("Promoting the secret containing the keypair used to sign bound service account tokens to the operand namespace.")
 		} else {
 			klog.V(2).Info("Promotion of the secret containing the keypair used to sign bound service account tokens is pending distribution of its public key to master nodes.")
 		}
 	}
-	if !syncRequired {
+	if !syncAllowed {
 		return nil
 	}
 	_, _, err = resourceapply.SyncSecret(c.secretClient, c.eventRecorder,
@@ -357,8 +362,8 @@ func (c *BoundSATokenSignerController) eventHandler() cache.ResourceEventHandler
 	}
 }
 
-// newSigningSecret creates a new secret populated with a new keypair.
-func newSigningSecret() (*corev1.Secret, error) {
+// newNextSigningSecret creates a new secret populated with a new keypair.
+func newNextSigningSecret() (*corev1.Secret, error) {
 	rsaKey, err := rsa.GenerateKey(rand.Reader, keySize)
 	if err != nil {
 		return nil, err