Always align documented values from Authentication (#970)

Originating issue: [IBMPrivateCloud/roadmap#65335](https://github.ibm.com/IBMPrivateCloud/roadmap/issues/65335)

Always updates the following values in the `platform-auth-idp` in order to better support various backup and restore behaviors:

* "ROKS_URL"
* "ROKS_USER_PREFIX"
* "ROKS_ENABLED"
* "BOOTSTRAP_USERID"
* "CLAIMS_SUPPORTED"
* "CLAIMS_MAP"
* "SCOPE_CLAIM"
* "NONCE_ENABLED"
* "PREFERRED_LOGIN"
* "OIDC_ISSUER_URL"
* "PROVIDER_ISSUER_URL"
* "CLUSTER_NAME"

---------

Signed-off-by: Rob Hundley <[email protected]>
Co-authored-by: Tirumala Mannaru <[email protected]>

Author: rwhundley

controllers/operator/configmap.go

@@ -281,6 +281,12 @@ func updatesValuesWhen(matches matcherFunc, keys ...string) (fn func(*corev1.Con
 	}
 }
 
+func updatesAlways(keys ...string) (fn func(*corev1.ConfigMap, *corev1.ConfigMap) bool) {
+	return func(observed, updates *corev1.ConfigMap) bool {
+		return updateFields(observed, updates, keys...)
+	}
+}
+
 func replaceOIDCClientRegistrationJob(ctx context.Context, cl client.Client, authCR *operatorv1alpha1.Authentication) (err error) {
 	job := &batchv1.Job{
 		ObjectMeta: metav1.ObjectMeta{
@@ -400,6 +406,20 @@ func updateOAuthClientConfigMap(observed, generated *corev1.ConfigMap) (updated
 
 func updatePlatformAuthIDP(observed, generated *corev1.ConfigMap) (updated bool, err error) {
 	updateFns := []func(*corev1.ConfigMap, *corev1.ConfigMap) bool{
+		updatesAlways(
+			"ROKS_URL",
+			"ROKS_USER_PREFIX",
+			"ROKS_ENABLED",
+			"BOOTSTRAP_USERID",
+			"CLAIMS_SUPPORTED",
+			"CLAIMS_MAP",
+			"SCOPE_CLAIM",
+			"NONCE_ENABLED",
+			"PREFERRED_LOGIN",
+			"OIDC_ISSUER_URL",
+			"PROVIDER_ISSUER_URL",
+			"CLUSTER_NAME",
+		),
 		updatesValuesWhen(observedKeyValueSetTo("OS_TOKEN_LENGTH", "45"),
 			"OS_TOKEN_LENGTH"),
 		updatesValuesWhen(observedKeyValueContains("IDENTITY_MGMT_URL", "127.0.0.1"),
@@ -415,15 +435,6 @@ func updatePlatformAuthIDP(observed, generated *corev1.ConfigMap) (updated bool,
 			"IDENTITY_PROVIDER_URL"),
 		updatesValuesWhen(not(observedKeySet("LDAP_RECURSIVE_SEARCH")),
 			"LDAP_RECURSIVE_SEARCH"),
-		updatesValuesWhen(not(observedKeySet("CLAIMS_SUPPORTED")),
-			"CLAIMS_SUPPORTED",
-			"CLAIMS_MAP",
-			"SCOPE_CLAIM",
-			"BOOTSTRAP_USERID"),
-		updatesValuesWhen(not(observedKeySet("PROVIDER_ISSUER_URL")),
-			"PROVIDER_ISSUER_URL"),
-		updatesValuesWhen(not(observedKeySet("PREFERRED_LOGIN")),
-			"PREFERRED_LOGIN"),
 		updatesValuesWhen(not(observedKeyValueSetTo("DEFAULT_LOGIN", generated.Data["DEFAULT_LOGIN"])),
 			"DEFAULT_LOGIN"),
 		updatesValuesWhen(not(observedKeyValueSetTo("MASTER_HOST", generated.Data["MASTER_HOST"])),
@@ -462,10 +473,6 @@ func updatePlatformAuthIDP(observed, generated *corev1.ConfigMap) (updated bool,
 			"LDAP_CTX_POOL_PREFERREDSIZE"),
 	}
 
-	if v, ok := generated.Data["ROKS_URL"]; ok {
-		updateFns = append(updateFns, updatesValuesWhen(
-			not(observedKeyValueSetTo("ROKS_URL", v)), "ROKS_URL"))
-	}
 	if v, ok := generated.Data["IS_OPENSHIFT_ENV"]; ok {
 		updateFns = append(updateFns, updatesValuesWhen(
 			not(observedKeyValueSetTo("IS_OPENSHIFT_ENV", v)), "IS_OPENSHIFT_ENV"))
@@ -520,6 +527,8 @@ func generateAuthIdpConfigMap(ctx context.Context, cl client.Client, authCR *ope
 	var desiredRoksUrl string
 	if authCR.Spec.Config.ROKSEnabled && !isOSEnv {
 		reqLogger.Info(".spec.config.roksEnabled is set to true, but workload does not appear to be running on OpenShift; disabling in ConfigMap")
+	} else if authCR.Spec.Config.ROKSEnabled && authCR.Spec.Config.ROKSURL != "https://roks.domain.name:443" {
+		desiredRoksUrl = authCR.Spec.Config.ROKSURL
 	} else if authCR.Spec.Config.ROKSEnabled {
 		if desiredRoksUrl, err = readROKSURL(ctx); err != nil {
 			reqLogger.Error(err, "Failed to get issuer URL")

controllers/operator/configmap_test.go

@@ -547,31 +547,10 @@ var _ = Describe("ConfigMap handling", func() {
 					"LDAP_RECURSIVE_SEARCH",
 					[]string{"LDAP_RECURSIVE_SEARCH"},
 				},
-				{
-					"CLAIMS_SUPPORTED",
-					[]string{
-						"CLAIMS_SUPPORTED",
-						"CLAIMS_MAP",
-						"SCOPE_CLAIM",
-						"BOOTSTRAP_USERID",
-					},
-				},
 				{
 					"DEFAULT_LOGIN",
 					[]string{"DEFAULT_LOGIN"},
 				},
-				{
-					"PROVIDER_ISSUER_URL",
-					[]string{
-						"PROVIDER_ISSUER_URL",
-					},
-				},
-				{
-					"PREFERRED_LOGIN",
-					[]string{
-						"PREFERRED_LOGIN",
-					},
-				},
 				{
 					"DB_CONNECT_TIMEOUT",
 					[]string{
@@ -654,7 +633,46 @@ var _ = Describe("ConfigMap handling", func() {
 			}
 
 			for _, test := range updateOnNotSetKeys {
-				observed := getObserved()
+				observed := &corev1.ConfigMap{}
+				Expect(generateAuthIdpConfigMap(ctx, r.Client, authCR, ibmcloudClusterInfo, observed)).
+					To(Succeed())
+				generated := &corev1.ConfigMap{}
+				err := generateAuthIdpConfigMap(ctx, r.Client, authCR, ibmcloudClusterInfo, generated)
+				Expect(err).NotTo(HaveOccurred())
+				updated, err = updatePlatformAuthIDP(observed, generated)
+				Expect(err).ToNot(HaveOccurred())
+				Expect(updated).To(BeFalse())
+				setDummyData(test.primaryKey, test.keys, observed)
+				updated, err = updatePlatformAuthIDP(observed, generated)
+				Expect(err).NotTo(HaveOccurred())
+				Expect(updated).To(BeTrue())
+				for _, k := range test.keys {
+					Expect(observed.Data[k]).To(Equal(generated.Data[k]))
+				}
+			}
+		})
+
+		It("sets fields when the values do not match the expected value", func() {
+			updateOnNotUpToDate := []struct {
+				primaryKey string
+				keys       []string
+			}{
+				{
+					"DEFAULT_LOGIN",
+					[]string{"DEFAULT_LOGIN"},
+				},
+			}
+
+			setDummyData := func(pkey string, keys []string, o *corev1.ConfigMap) {
+				dummyValue := "dummy"
+				for _, k := range keys {
+					o.Data[k] = dummyValue
+				}
+			}
+			for _, test := range updateOnNotUpToDate {
+				observed := &corev1.ConfigMap{}
+				Expect(generateAuthIdpConfigMap(ctx, r.Client, authCR, ibmcloudClusterInfo, observed)).
+					To(Succeed())
 				generated := &corev1.ConfigMap{}
 				err := generateAuthIdpConfigMap(ctx, r.Client, authCR, ibmcloudClusterInfo, generated)
 				Expect(err).NotTo(HaveOccurred())
@@ -694,7 +712,9 @@ var _ = Describe("ConfigMap handling", func() {
 		})
 
 		It("replaces value in OS_TOKEN_LENGTH only when it is set to 45", func() {
-			observed := getObserved()
+			observed := &corev1.ConfigMap{}
+			Expect(generateAuthIdpConfigMap(ctx, r.Client, authCR, ibmcloudClusterInfo, observed)).
+				To(Succeed())
 			// Set the keys in observed to contain localhost IP
 			k := "OS_TOKEN_LENGTH"
 			observed.Data[k] = "24"
@@ -711,6 +731,45 @@ var _ = Describe("ConfigMap handling", func() {
 			Expect(updated).To(BeTrue())
 			Expect(observed.Data[k]).To(Equal(generated.Data[k]))
 		})
+
+		It("always replaces certain values if they differ", func() {
+			updateAlways := []string{
+				"ROKS_URL",
+				"ROKS_USER_PREFIX",
+				"ROKS_ENABLED",
+				"BOOTSTRAP_USERID",
+				"CLAIMS_SUPPORTED",
+				"CLAIMS_MAP",
+				"SCOPE_CLAIM",
+				"NONCE_ENABLED",
+				"PREFERRED_LOGIN",
+				"OIDC_ISSUER_URL",
+				"PROVIDER_ISSUER_URL",
+				"CLUSTER_NAME",
+			}
+
+			setDummyData := func(k string, o *corev1.ConfigMap) {
+				dummyValue := "dummy"
+				o.Data[k] = dummyValue
+			}
+			for _, test := range updateAlways {
+				observed := getObserved()
+				Expect(generateAuthIdpConfigMap(ctx, r.Client, authCR, ibmcloudClusterInfo, observed)).
+					To(Succeed())
+				generated := &corev1.ConfigMap{}
+				err := generateAuthIdpConfigMap(ctx, r.Client, authCR, ibmcloudClusterInfo, generated)
+				Expect(err).NotTo(HaveOccurred())
+				updated, err = updatePlatformAuthIDP(observed, generated)
+				Expect(err).ToNot(HaveOccurred())
+				Expect(updated).To(BeFalse())
+				setDummyData(test, observed)
+				Expect(observed.Data[test]).To(Equal("dummy"))
+				updated, err = updatePlatformAuthIDP(observed, generated)
+				Expect(err).NotTo(HaveOccurred())
+				Expect(updated).To(BeTrue())
+				Expect(observed.Data[test]).To(Equal(generated.Data[test]))
+			}
+		})
 	})
 
 	Describe("oauth-client-map handling", func() {