Update secrets when mgmt configmap changes

internal/k8s/controller.go

@@ -883,6 +883,7 @@
 	var isNGINXConfigValid bool
 	var mgmtConfigHasWarnings bool
 	var mgmtErr error
+	var reloadNginx bool
 
 	if lbc.configMap != nil {
 		cfgParams, isNGINXConfigValid = configs.ParseConfigMap(ctx, lbc.configMap, lbc.isNginxPlus, lbc.appProtectEnabled, lbc.appProtectDosEnabled, lbc.configuration.isTLSPassthroughEnabled, lbc.recorder)
@@ -892,6 +893,15 @@
 		if mgmtErr != nil {
 			nl.Errorf(lbc.Logger, "configmap %s/%s: %v", lbc.mgmtConfigMap.GetNamespace(), lbc.mgmtConfigMap.GetName(), mgmtErr)
 		}
+		// update special license secret in mgmtConfigParams
+		if mgmtCfgParams.Secrets.License != "" {
+			secret, err := lbc.client.CoreV1().Secrets(lbc.mgmtConfigMap.GetNamespace()).Get(context.TODO(), mgmtCfgParams.Secrets.License, meta_v1.GetOptions{})
+			if err != nil {
+				nl.Errorf(lbc.Logger, "secret %s/%s: %v", lbc.mgmtConfigMap.GetNamespace(), mgmtCfgParams.Secrets.License, err)
+			}
+			lbc.specialSecrets.licenseSecret = fmt.Sprintf("%s/%s", secret.Namespace, secret.Name)
+			lbc.handleSpecialSecretUpdate(secret, reloadNginx)
+		}
 		// update special CA secret in mgmtConfigParams
 		if mgmtCfgParams.Secrets.TrustedCert != "" {
 			secret, err := lbc.client.CoreV1().Secrets(lbc.mgmtConfigMap.GetNamespace()).Get(context.TODO(), mgmtCfgParams.Secrets.TrustedCert, meta_v1.GetOptions{})
@@ -901,6 +911,17 @@
 			if _, hasCRL := secret.Data[configs.CACrlKey]; hasCRL {
 				mgmtCfgParams.Secrets.TrustedCRL = secret.Name
 			}
+			lbc.specialSecrets.trustedCertSecret = fmt.Sprintf("%s/%s", secret.Namespace, secret.Name)
+			lbc.handleSpecialSecretUpdate(secret, reloadNginx)
+		}
+		// update special ClientAuth secret in mgmtConfigParams
+		if mgmtCfgParams.Secrets.ClientAuth != "" {
+			secret, err := lbc.client.CoreV1().Secrets(lbc.mgmtConfigMap.GetNamespace()).Get(context.TODO(), mgmtCfgParams.Secrets.ClientAuth, meta_v1.GetOptions{})
+			if err != nil {
+				nl.Errorf(lbc.Logger, "secret %s/%s: %v", lbc.mgmtConfigMap.GetNamespace(), mgmtCfgParams.Secrets.ClientAuth, err)
+			}
+			lbc.specialSecrets.clientAuthSecret = fmt.Sprintf("%s/%s", secret.Namespace, secret.Name)
+			lbc.handleSpecialSecretUpdate(secret, reloadNginx)
 		}
 	}
 
@@ -1769,7 +1790,8 @@
 	lbc.secretStore.AddOrUpdateSecret(secret)
 
 	if lbc.isSpecialSecret(key) {
-		lbc.handleSpecialSecretUpdate(secret)
+		reloadNginx := true
+		lbc.handleSpecialSecretUpdate(secret, reloadNginx)
 		// we don't return here in case the special secret is also used in resources.
 	}
 
@@ -1828,25 +1850,22 @@
 	warnings, addOrUpdateErr = lbc.configurator.AddOrUpdateResources(resourceExes, !lbc.configurator.DynamicSSLReloadEnabled())
 	if addOrUpdateErr != nil {
 		nl.Errorf(lbc.Logger, "Error when updating Secret %v: %v", secretNsName, addOrUpdateErr)
-		lbc.recorder.Eventf(secret, api_v1.EventTypeWarning, "UpdatedWithError", "%v was updated, but not applied: %v", secretNsName, addOrUpdateErr)
+		lbc.recorder.Eventf(lbc.metadata.pod, api_v1.EventTypeWarning, "UpdatedWithError", "%v was updated, but not applied: %v", secretNsName, addOrUpdateErr)
 	}
 
 	lbc.updateResourcesStatusAndEvents(resources, warnings, addOrUpdateErr)
 }
 
-func (lbc *LoadBalancerController) validationTLSSpecialSecret(secret *api_v1.Secret, secretName string, secretList *[]string) {
-	secretNsName := generateSecretNSName(secret)
-
+func (lbc *LoadBalancerController) validationTLSSpecialSecret(secret *api_v1.Secret, secretName string, secretList *[]string) error {
 	err := secrets.ValidateTLSSecret(secret)
 	if err != nil {
-		nl.Errorf(lbc.Logger, "Couldn't validate the special Secret %v: %v", secretNsName, err)
-		lbc.recorder.Eventf(secret, api_v1.EventTypeWarning, "Rejected", "the special Secret %v was rejected, using the previous version: %v", secretNsName, err)
-		return
+		return err
 	}
 	*secretList = append(*secretList, secretName)
+	return nil
 }
 
-func (lbc *LoadBalancerController) handleSpecialSecretUpdate(secret *api_v1.Secret) {
+func (lbc *LoadBalancerController) handleSpecialSecretUpdate(secret *api_v1.Secret, reload bool) {
 	var specialTLSSecretsToUpdate []string
 	secretNsName := generateSecretNSName(secret)
 
@@ -1860,6 +1879,12 @@
 		return
 	}
 
+	// When the MGMT Configmap updates, we don't need to reload here, we are reloading in updateAllConfigs().
+	if !reload {
+		lbc.recorder.Eventf(lbc.metadata.pod, api_v1.EventTypeNormal, "SecretUpdated", "the special Secret %v was updated", secretNsName)
+		return
+	}
+
 	// reload nginx when the TLS special secrets are updated
 	switch secretNsName {
 	case lbc.specialSecrets.licenseSecret:
@@ -1881,7 +1906,7 @@
 		}
 	}
 
-	lbc.recorder.Eventf(secret, api_v1.EventTypeNormal, "Updated", "the special Secret %v was updated", secretNsName)
+	lbc.recorder.Eventf(lbc.metadata.pod, api_v1.EventTypeNormal, "SecretUpdated", "the special Secret %v was updated", secretNsName)
 }
 
 // writeSpecialSecrets generates content and writes the secret to disk
@@ -1904,10 +1929,20 @@
 
 func (lbc *LoadBalancerController) specialSecretValidation(secretNsName string, secret *api_v1.Secret, specialTLSSecretsToUpdate *[]string) bool {
 	if secretNsName == lbc.specialSecrets.defaultServerSecret {
-		lbc.validationTLSSpecialSecret(secret, configs.DefaultServerSecretFileName, specialTLSSecretsToUpdate)
+		err := lbc.validationTLSSpecialSecret(secret, configs.DefaultServerSecretFileName, specialTLSSecretsToUpdate)
+		if err != nil {
+			nl.Errorf(lbc.Logger, "Couldn't validate the special Secret %v: %v", secretNsName, err)
+			lbc.recorder.Eventf(lbc.metadata.pod, api_v1.EventTypeWarning, "Rejected", "the special Secret %v was rejected, using the previous version: %v", secretNsName, err)
+			return false
+		}
 	}
 	if secretNsName == lbc.specialSecrets.wildcardTLSSecret {
-		lbc.validationTLSSpecialSecret(secret, configs.WildcardSecretFileName, specialTLSSecretsToUpdate)
+		err := lbc.validationTLSSpecialSecret(secret, configs.WildcardSecretFileName, specialTLSSecretsToUpdate)
+		if err != nil {
+			nl.Errorf(lbc.Logger, "Couldn't validate the special Secret %v: %v", secretNsName, err)
+			lbc.recorder.Eventf(lbc.metadata.pod, api_v1.EventTypeWarning, "Rejected", "the special Secret %v was rejected, using the previous version: %v", secretNsName, err)
+			return false
+		}
 	}
 	if secretNsName == lbc.specialSecrets.licenseSecret {
 		err := secrets.ValidateLicenseSecret(secret)
@@ -1926,7 +1961,12 @@
 		}
 	}
 	if secretNsName == lbc.specialSecrets.clientAuthSecret {
-		lbc.validationTLSSpecialSecret(secret, configs.ClientAuthCertSecretFileName, specialTLSSecretsToUpdate)
+		err := lbc.validationTLSSpecialSecret(secret, configs.ClientAuthCertSecretFileName, specialTLSSecretsToUpdate)
+		if err != nil {
+			nl.Errorf(lbc.Logger, "Couldn't validate the special Secret %v: %v", secretNsName, err)
+			lbc.recorder.Eventf(lbc.metadata.pod, api_v1.EventTypeWarning, "Rejected", "the special Secret %v was rejected, using the previous version: %v", secretNsName, err)
+			return false
+		}
 	}
 	return true
 }

tests/data/mgmt-configmap-keys/plus-token-name-keys.yaml

@@ -0,0 +1,7 @@
+apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: nginx-config-mgmt
+  namespace: nginx-ingress
+data:
+  license-token-secret-name: "license-token-changed"

tests/suite/test_mgmt_configmap_keys.py

@@ -0,0 +1,106 @@
+import pytest
+from settings import TEST_DATA
+from suite.utils.resources_utils import (
+    create_license,
+    ensure_connection_to_public_endpoint,
+    get_events_for_object,
+    get_first_pod_name,
+    get_reload_count,
+    is_secret_present,
+    replace_configmap_from_yaml,
+    wait_before_test,
+)
+
+
+def assert_event(event_list, event_type, reason, message_substring):
+    """
+    Assert that an event with specific type, reason, and message substring exists.
+
+    :param event_list: List of events
+    :param event_type: 'Normal' or 'Warning'
+    :param reason: Event reason
+    :param message_substring: Substring expected in the event message
+    """
+    for event in event_list:
+        if event.type == event_type and event.reason == reason and message_substring in event.message:
+            return
+    assert (
+        False
+    ), f"Expected event with type '{event_type}', reason '{reason}', and message containing '{message_substring}' not found."
+
+
+@pytest.mark.skip_for_nginx_oss
+@pytest.mark.ingresses
+@pytest.mark.smoke
+class TestMGMTConfigMap:
+    @pytest.mark.parametrize(
+        "ingress_controller",
+        [
+            pytest.param(
+                {"extra_args": ["-enable-prometheus-metrics"]},
+            )
+        ],
+        indirect=["ingress_controller"],
+    )
+    def test_mgmt_configmap_events(
+        self,
+        cli_arguments,
+        kube_apis,
+        ingress_controller_prerequisites,
+        ingress_controller,
+        ingress_controller_endpoint,
+    ):
+        ensure_connection_to_public_endpoint(
+            ingress_controller_endpoint.public_ip,
+            ingress_controller_endpoint.port,
+            ingress_controller_endpoint.port_ssl,
+        )
+        ic_pod_name = get_first_pod_name(kube_apis.v1, ingress_controller_prerequisites.namespace)
+        metrics_url = (
+            f"http://{ingress_controller_endpoint.public_ip}:{ingress_controller_endpoint.metrics_port}/metrics"
+        )
+
+        print("Step 1: get reload count")
+        reload_count = get_reload_count(metrics_url)
+
+        wait_before_test(1)
+        print(f"Step 1a: initial reload count is {reload_count}")
+
+        print("Step 2: create duplicate existing secret with new name")
+        license_name = create_license(
+            kube_apis.v1,
+            ingress_controller_prerequisites.namespace,
+            cli_arguments["plus-jwt"],
+            license_token_name="license-token-changed",
+        )
+        assert is_secret_present(kube_apis.v1, license_name, ingress_controller_prerequisites.namespace)
+
+        print("Step 3: update the ConfigMap/license-token-secret-name to the new secret")
+        replace_configmap_from_yaml(
+            kube_apis.v1,
+            "nginx-config-mgmt",
+            ingress_controller_prerequisites.namespace,
+            f"{TEST_DATA}/mgmt-configmap-keys/plus-token-name-keys.yaml",
+        )
+
+        wait_before_test()
+
+        print("Step 4: check reload count has incremented")
+        new_reload_count = get_reload_count(metrics_url)
+        print(f"Step 4a: new reload count is {new_reload_count}")
+        assert new_reload_count > reload_count
+
+        print("Step 5: check pod for SecretUpdated event")
+        events = get_events_for_object(
+            kube_apis.v1,
+            ingress_controller_prerequisites.namespace,
+            ic_pod_name,
+        )
+
+        # Assert that the 'SecretUpdated' event is present
+        assert_event(
+            events,
+            "Normal",
+            "SecretUpdated",
+            f"the special Secret {ingress_controller_prerequisites.namespace}/{license_name} was updated",
+        )