Skip to content

Commit

Permalink
Update secrets when mgmt configmap changes (#6947)
Browse files Browse the repository at this point in the history
  • Loading branch information
pdabelf5 authored Dec 10, 2024
1 parent 552992d commit 15403da
Show file tree
Hide file tree
Showing 3 changed files with 166 additions and 13 deletions.
66 changes: 53 additions & 13 deletions internal/k8s/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -883,6 +883,7 @@ func (lbc *LoadBalancerController) updateAllConfigs() {
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)
Expand All @@ -892,6 +893,15 @@ func (lbc *LoadBalancerController) updateAllConfigs() {
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{})
Expand All @@ -901,6 +911,17 @@ func (lbc *LoadBalancerController) updateAllConfigs() {
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)
}
}

Expand Down Expand Up @@ -1769,7 +1790,8 @@ func (lbc *LoadBalancerController) syncSecret(task task) {
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.
}

Expand Down Expand Up @@ -1828,25 +1850,22 @@ func (lbc *LoadBalancerController) handleSecretUpdate(secret *api_v1.Secret, res
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)

Expand All @@ -1860,6 +1879,12 @@ func (lbc *LoadBalancerController) handleSpecialSecretUpdate(secret *api_v1.Secr
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:
Expand All @@ -1881,7 +1906,7 @@ func (lbc *LoadBalancerController) handleSpecialSecretUpdate(secret *api_v1.Secr
}
}

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
Expand All @@ -1904,10 +1929,20 @@ func (lbc *LoadBalancerController) writeSpecialSecrets(secret *api_v1.Secret, se

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)
Expand All @@ -1926,7 +1961,12 @@ func (lbc *LoadBalancerController) specialSecretValidation(secretNsName string,
}
}
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
}
Expand Down
7 changes: 7 additions & 0 deletions tests/data/mgmt-configmap-keys/plus-token-name-keys.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
apiVersion: v1
kind: ConfigMap
metadata:
name: nginx-config-mgmt
namespace: nginx-ingress
data:
license-token-secret-name: "license-token-changed"
106 changes: 106 additions & 0 deletions tests/suite/test_mgmt_configmap_keys.py
Original file line number Diff line number Diff line change
@@ -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",
)

0 comments on commit 15403da

Please sign in to comment.