Skip to content

Commit

Permalink
refactor handleSpecialSecretUpdate (#6875)
Browse files Browse the repository at this point in the history
  • Loading branch information
Jim Ryan authored Nov 28, 2024
1 parent 5b5e052 commit ba34c0e
Show file tree
Hide file tree
Showing 2 changed files with 67 additions and 43 deletions.
63 changes: 27 additions & 36 deletions internal/configs/configurator.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,7 @@ func (cnf *Configurator) AddOrUpdateIngress(ingEx *IngressEx) (Warnings, error)
return warnings, fmt.Errorf("error adding or updating ingress %v/%v: %w", ingEx.Ingress.Namespace, ingEx.Ingress.Name, err)
}

if err := cnf.reload(nginx.ReloadForOtherUpdate); err != nil {
if err := cnf.Reload(nginx.ReloadForOtherUpdate); err != nil {
return warnings, fmt.Errorf("error reloading NGINX for %v/%v: %w", ingEx.Ingress.Namespace, ingEx.Ingress.Name, err)
}

Expand Down Expand Up @@ -430,7 +430,7 @@ func (cnf *Configurator) AddOrUpdateMergeableIngress(mergeableIngs *MergeableIng
return warnings, fmt.Errorf("error when adding or updating ingress %v/%v: %w", mergeableIngs.Master.Ingress.Namespace, mergeableIngs.Master.Ingress.Name, err)
}

if err := cnf.reload(nginx.ReloadForOtherUpdate); err != nil {
if err := cnf.Reload(nginx.ReloadForOtherUpdate); err != nil {
return warnings, fmt.Errorf("error reloading NGINX for %v/%v: %w", mergeableIngs.Master.Ingress.Namespace, mergeableIngs.Master.Ingress.Name, err)
}

Expand Down Expand Up @@ -585,7 +585,7 @@ func (cnf *Configurator) AddOrUpdateVirtualServer(virtualServerEx *VirtualServer
cnf.EnableReloads()
}

if err := cnf.reload(nginx.ReloadForOtherUpdate); err != nil {
if err := cnf.Reload(nginx.ReloadForOtherUpdate); err != nil {
return warnings, fmt.Errorf("error reloading NGINX for VirtualServer %v/%v: %w", virtualServerEx.VirtualServer.Namespace, virtualServerEx.VirtualServer.Name, err)
}

Expand Down Expand Up @@ -656,7 +656,7 @@ func (cnf *Configurator) AddOrUpdateVirtualServers(virtualServerExes []*VirtualS
allWeightUpdates = append(allWeightUpdates, weightUpdates...)
}

if err := cnf.reload(nginx.ReloadForOtherUpdate); err != nil {
if err := cnf.Reload(nginx.ReloadForOtherUpdate); err != nil {
return allWarnings, fmt.Errorf("error when reloading NGINX when updating Policy: %w", err)
}

Expand Down Expand Up @@ -741,7 +741,7 @@ func (cnf *Configurator) AddOrUpdateTransportServer(transportServerEx *Transport
if err != nil {
return nil, fmt.Errorf("error adding or updating TransportServer %v/%v: %w", transportServerEx.TransportServer.Namespace, transportServerEx.TransportServer.Name, err)
}
if err := cnf.reload(nginx.ReloadForOtherUpdate); err != nil {
if err := cnf.Reload(nginx.ReloadForOtherUpdate); err != nil {
return nil, fmt.Errorf("error reloading NGINX for TransportServer %v/%v: %w", transportServerEx.TransportServer.Namespace, transportServerEx.TransportServer.Name, err)
}
return warnings, nil
Expand Down Expand Up @@ -909,7 +909,7 @@ func (cnf *Configurator) AddOrUpdateResources(resources ExtendedResources, reloa
}

if configsChanged || reloadIfUnchanged {
if err := cnf.reload(nginx.ReloadForOtherUpdate); err != nil {
if err := cnf.Reload(nginx.ReloadForOtherUpdate); err != nil {
return nil, fmt.Errorf("error when reloading NGINX when updating resources: %w", err)
}
}
Expand All @@ -923,24 +923,14 @@ func (cnf *Configurator) addOrUpdateTLSSecret(secret *api_v1.Secret) string {
}

// AddOrUpdateSpecialTLSSecrets adds or updates a file with a TLS cert and a key from a Special TLS Secret (eg. DefaultServerSecret, WildcardTLSSecret).
func (cnf *Configurator) AddOrUpdateSpecialTLSSecrets(secret *api_v1.Secret, secretNames []string) error {
func (cnf *Configurator) AddOrUpdateSpecialTLSSecrets(secret *api_v1.Secret, secretNames []string) {
l := nl.LoggerFromContext(cnf.CfgParams.Context)
nl.Debugf(l, "AddOrUpdateSpecialTLSSecrets: secrets [%v]", secretNames)
data := GenerateCertAndKeyFileContent(secret)

for _, secretName := range secretNames {
cnf.nginxManager.CreateSecret(secretName, data, nginx.ReadWriteOnlyFileMode)
}

if !cnf.DynamicSSLReloadEnabled() {
if err := cnf.reload(nginx.ReloadForOtherUpdate); err != nil {
return fmt.Errorf("error when reloading NGINX when updating the special Secrets: %w", err)
}
} else {
nl.Debugf(l, "Skipping reload for %d special Secrets", len(secretNames))
}

return nil
}

// GenerateCertAndKeyFileContent generates a pem file content from the TLS secret.
Expand Down Expand Up @@ -979,7 +969,7 @@ func (cnf *Configurator) DeleteIngress(key string, skipReload bool) error {
}

if !skipReload {
if err := cnf.reload(nginx.ReloadForOtherUpdate); err != nil {
if err := cnf.Reload(nginx.ReloadForOtherUpdate); err != nil {
return fmt.Errorf("error when removing ingress %v: %w", key, err)
}
}
Expand All @@ -1002,7 +992,7 @@ func (cnf *Configurator) DeleteVirtualServer(key string, skipReload bool) error
}

if !skipReload {
if err := cnf.reload(nginx.ReloadForOtherUpdate); err != nil {
if err := cnf.Reload(nginx.ReloadForOtherUpdate); err != nil {
return fmt.Errorf("error when removing VirtualServer %v: %w", key, err)
}
}
Expand All @@ -1021,7 +1011,7 @@ func (cnf *Configurator) DeleteTransportServer(key string) error {
return fmt.Errorf("error when removing TransportServer %v: %w", key, err)
}

err = cnf.reload(nginx.ReloadForOtherUpdate)
err = cnf.Reload(nginx.ReloadForOtherUpdate)
if err != nil {
return fmt.Errorf("error when removing TransportServer %v: %w", key, err)
}
Expand Down Expand Up @@ -1070,7 +1060,7 @@ func (cnf *Configurator) UpdateEndpoints(ingExes []*IngressEx) error {
return nil
}

if err := cnf.reload(nginx.ReloadForEndpointsUpdate); err != nil {
if err := cnf.Reload(nginx.ReloadForEndpointsUpdate); err != nil {
return fmt.Errorf("error reloading NGINX when updating endpoints: %w", err)
}

Expand Down Expand Up @@ -1105,7 +1095,7 @@ func (cnf *Configurator) UpdateEndpointsMergeableIngress(mergeableIngresses []*M
return nil
}

if err := cnf.reload(nginx.ReloadForEndpointsUpdate); err != nil {
if err := cnf.Reload(nginx.ReloadForEndpointsUpdate); err != nil {
return fmt.Errorf("error reloading NGINX when updating endpoints for %v: %w", mergeableIngresses, err)
}

Expand Down Expand Up @@ -1138,7 +1128,7 @@ func (cnf *Configurator) UpdateEndpointsForVirtualServers(virtualServerExes []*V
return nil
}

if err := cnf.reload(nginx.ReloadForEndpointsUpdate); err != nil {
if err := cnf.Reload(nginx.ReloadForEndpointsUpdate); err != nil {
return fmt.Errorf("error reloading NGINX when updating endpoints: %w", err)
}

Expand Down Expand Up @@ -1185,7 +1175,7 @@ func (cnf *Configurator) UpdateEndpointsForTransportServers(transportServerExes
nl.Debug(l, "No need to reload nginx")
return nil
}
if err := cnf.reload(nginx.ReloadForEndpointsUpdate); err != nil {
if err := cnf.Reload(nginx.ReloadForEndpointsUpdate); err != nil {
return fmt.Errorf("error reloading NGINX when updating endpoints: %w", err)
}
return nil
Expand Down Expand Up @@ -1272,7 +1262,8 @@ func (cnf *Configurator) DisableReloads() {
cnf.isReloadsEnabled = false
}

func (cnf *Configurator) reload(isEndpointsUpdate bool) error {
// Reload reloads nginx if reloads is enabled
func (cnf *Configurator) Reload(isEndpointsUpdate bool) error {
if !cnf.isReloadsEnabled {
return nil
}
Expand Down Expand Up @@ -1398,7 +1389,7 @@ func (cnf *Configurator) UpdateConfig(cfgParams *ConfigParams, resources Extende
}

cnf.nginxManager.SetOpenTracing(mainCfg.OpenTracingLoadModule)
if err := cnf.reload(nginx.ReloadForOtherUpdate); err != nil {
if err := cnf.Reload(nginx.ReloadForOtherUpdate); err != nil {
return allWarnings, fmt.Errorf("error when updating config from ConfigMap: %w", err)
}

Expand All @@ -1414,7 +1405,7 @@ func (cnf *Configurator) ReloadForBatchUpdates(batchReloadsEnabled bool) error {
if !batchReloadsEnabled {
return nil
}
if err := cnf.reload(nginx.ReloadForOtherUpdate); err != nil {
if err := cnf.Reload(nginx.ReloadForOtherUpdate); err != nil {
return fmt.Errorf("error when reloading NGINX after a batch event: %w", err)
}
return nil
Expand All @@ -1439,7 +1430,7 @@ func (cnf *Configurator) UpdateVirtualServers(updatedVSExes []*VirtualServerEx,
}
}

if err := cnf.reload(nginx.ReloadForOtherUpdate); err != nil {
if err := cnf.Reload(nginx.ReloadForOtherUpdate); err != nil {
errList = append(errList, fmt.Errorf("error when updating VirtualServer: %w", err))
}

Expand Down Expand Up @@ -1467,7 +1458,7 @@ func (cnf *Configurator) UpdateTransportServers(updatedTSExes []*TransportServer
}
}

if err := cnf.reload(nginx.ReloadForOtherUpdate); err != nil {
if err := cnf.Reload(nginx.ReloadForOtherUpdate); err != nil {
errList = append(errList, fmt.Errorf("error when updating TransportServers: %w", err))
}

Expand All @@ -1484,7 +1475,7 @@ func (cnf *Configurator) BatchDeleteVirtualServers(deletedKeys []string) []error
}
}

if err := cnf.reload(nginx.ReloadForOtherUpdate); err != nil {
if err := cnf.Reload(nginx.ReloadForOtherUpdate); err != nil {
errList = append(errList, fmt.Errorf("error when reloading NGINX for deleted VirtualServers: %w", err))
}

Expand All @@ -1501,7 +1492,7 @@ func (cnf *Configurator) BatchDeleteIngresses(deletedKeys []string) []error {
}
}

if err := cnf.reload(nginx.ReloadForOtherUpdate); err != nil {
if err := cnf.Reload(nginx.ReloadForOtherUpdate); err != nil {
errList = append(errList, fmt.Errorf("error when reloading NGINX for deleted Ingresses: %w", err))
}

Expand Down Expand Up @@ -1677,7 +1668,7 @@ func (cnf *Configurator) AddOrUpdateSpiffeCerts(svidResponse *workloadapi.X509Co
cnf.nginxManager.CreateSecret(spiffeCertFileName, pemCerts, spiffeCertsFileMode)
cnf.nginxManager.CreateSecret(spiffeBundleFileName, pemBundle, spiffeCertsFileMode)

err = cnf.reload(nginx.ReloadForOtherUpdate)
err = cnf.Reload(nginx.ReloadForOtherUpdate)
if err != nil {
return fmt.Errorf("error when reloading NGINX when updating the SPIFFE Certs: %w", err)
}
Expand Down Expand Up @@ -1822,7 +1813,7 @@ func (cnf *Configurator) AddOrUpdateAppProtectResource(resource *unstructured.Un
return warnings, fmt.Errorf("error when updating %v %v/%v: %w", resource.GetKind(), resource.GetNamespace(), resource.GetName(), err)
}

err = cnf.reload(nginx.ReloadForOtherUpdate)
err = cnf.Reload(nginx.ReloadForOtherUpdate)
if err != nil {
return warnings, fmt.Errorf("error when reloading NGINX when updating %v %v/%v: %w", resource.GetKind(), resource.GetNamespace(), resource.GetName(), err)
}
Expand All @@ -1837,7 +1828,7 @@ func (cnf *Configurator) AddOrUpdateResourcesThatUseDosProtected(ingExes []*Ingr
return warnings, fmt.Errorf("error when updating resources that use Dos: %w", err)
}

err = cnf.reload(nginx.ReloadForOtherUpdate)
err = cnf.Reload(nginx.ReloadForOtherUpdate)
if err != nil {
return warnings, fmt.Errorf("error when updating resources that use Dos: %w", err)
}
Expand Down Expand Up @@ -1925,7 +1916,7 @@ func (cnf *Configurator) RefreshAppProtectUserSigs(
fmt.Fprintf(&builder, "app_protect_user_defined_signatures %s;\n", fName)
}
cnf.nginxManager.CreateAppProtectResourceFile(appProtectUserSigIndex, []byte(builder.String()))
return allWarnings, cnf.reload(nginx.ReloadForOtherUpdate)
return allWarnings, cnf.Reload(nginx.ReloadForOtherUpdate)
}

func appProtectDosPolicyFileName(namespace string, name string) string {
Expand Down Expand Up @@ -1965,7 +1956,7 @@ func (cnf *Configurator) AddInternalRouteConfig() error {
return fmt.Errorf("error when writing main Config: %w", err)
}
cnf.nginxManager.CreateMainConfig(mainCfgContent)
if err := cnf.reload(nginx.ReloadForOtherUpdate); err != nil {
if err := cnf.Reload(nginx.ReloadForOtherUpdate); err != nil {
return fmt.Errorf("error when reloading nginx: %w", err)
}
return nil
Expand Down
47 changes: 40 additions & 7 deletions internal/k8s/controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1781,20 +1781,53 @@ func (lbc *LoadBalancerController) handleSpecialSecretUpdate(secret *api_v1.Secr
var specialTLSSecretsToUpdate []string
secretNsName := generateSecretNSName(secret)

if ok := lbc.specialSecretValidation(secretNsName, secret, &specialTLSSecretsToUpdate); !ok {
// if not ok bail early
return
}

lbc.writeSpecialSecrets(secret, specialTLSSecretsToUpdate)

// reload nginx when the TLS special secrets are updated
switch secretNsName {
case lbc.specialSecrets.defaultServerSecret, lbc.specialSecrets.wildcardTLSSecret:
if ok := lbc.performDynamicSSLReload(secret); !ok {
return
}
}

lbc.recorder.Eventf(secret, api_v1.EventTypeNormal, "Updated", "the special Secret %v was updated", secretNsName)
}

func (lbc *LoadBalancerController) writeSpecialSecrets(secret *api_v1.Secret, specialTLSSecretsToUpdate []string) {
lbc.configurator.AddOrUpdateSpecialTLSSecrets(secret, specialTLSSecretsToUpdate)
}

func (lbc *LoadBalancerController) specialSecretValidation(secretNsName string, secret *api_v1.Secret, specialTLSSecretsToUpdate *[]string) bool {
if secretNsName == lbc.specialSecrets.defaultServerSecret {
lbc.validationTLSSpecialSecret(secret, configs.DefaultServerSecretFileName, &specialTLSSecretsToUpdate)
lbc.validationTLSSpecialSecret(secret, configs.DefaultServerSecretFileName, specialTLSSecretsToUpdate)
}
if secretNsName == lbc.specialSecrets.wildcardTLSSecret {
lbc.validationTLSSpecialSecret(secret, configs.WildcardSecretFileName, &specialTLSSecretsToUpdate)
lbc.validationTLSSpecialSecret(secret, configs.WildcardSecretFileName, specialTLSSecretsToUpdate)
}
return true
}

err := lbc.configurator.AddOrUpdateSpecialTLSSecrets(secret, specialTLSSecretsToUpdate)
if err != nil {
nl.Errorf(lbc.Logger, "Error when updating the special Secret %v: %v", secretNsName, err)
func (lbc *LoadBalancerController) performDynamicSSLReload(secret *api_v1.Secret) bool {
if !lbc.configurator.DynamicSSLReloadEnabled() {
return lbc.performNGINXReload(secret)
}
return true
}

func (lbc *LoadBalancerController) performNGINXReload(secret *api_v1.Secret) bool {
secretNsName := generateSecretNSName(secret)
if err := lbc.configurator.Reload(false); err != nil {
nl.Errorf(lbc.Logger, "error when reloading NGINX when updating the special Secrets: %v", err)
lbc.recorder.Eventf(secret, api_v1.EventTypeWarning, "UpdatedWithError", "the special Secret %v was updated, but not applied: %v", secretNsName, err)
return
return false
}
lbc.recorder.Eventf(secret, api_v1.EventTypeNormal, "Updated", "the special Secret %v was updated", secretNsName)
return true
}

func generateSecretNSName(secret *api_v1.Secret) string {
Expand Down

0 comments on commit ba34c0e

Please sign in to comment.