Skip to content

Commit ba34c0e

Browse files
author
Jim Ryan
authored
refactor handleSpecialSecretUpdate (nginx#6875)
1 parent 5b5e052 commit ba34c0e

File tree

2 files changed

+67
-43
lines changed

2 files changed

+67
-43
lines changed

‎internal/configs/configurator.go‎

Lines changed: 27 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -295,7 +295,7 @@ func (cnf *Configurator) AddOrUpdateIngress(ingEx *IngressEx) (Warnings, error)
295295
return warnings, fmt.Errorf("error adding or updating ingress %v/%v: %w", ingEx.Ingress.Namespace, ingEx.Ingress.Name, err)
296296
}
297297

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

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

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

@@ -585,7 +585,7 @@ func (cnf *Configurator) AddOrUpdateVirtualServer(virtualServerEx *VirtualServer
585585
cnf.EnableReloads()
586586
}
587587

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

@@ -656,7 +656,7 @@ func (cnf *Configurator) AddOrUpdateVirtualServers(virtualServerExes []*VirtualS
656656
allWeightUpdates = append(allWeightUpdates, weightUpdates...)
657657
}
658658

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

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

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

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

931931
for _, secretName := range secretNames {
932932
cnf.nginxManager.CreateSecret(secretName, data, nginx.ReadWriteOnlyFileMode)
933933
}
934-
935-
if !cnf.DynamicSSLReloadEnabled() {
936-
if err := cnf.reload(nginx.ReloadForOtherUpdate); err != nil {
937-
return fmt.Errorf("error when reloading NGINX when updating the special Secrets: %w", err)
938-
}
939-
} else {
940-
nl.Debugf(l, "Skipping reload for %d special Secrets", len(secretNames))
941-
}
942-
943-
return nil
944934
}
945935

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

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

1004994
if !skipReload {
1005-
if err := cnf.reload(nginx.ReloadForOtherUpdate); err != nil {
995+
if err := cnf.Reload(nginx.ReloadForOtherUpdate); err != nil {
1006996
return fmt.Errorf("error when removing VirtualServer %v: %w", key, err)
1007997
}
1008998
}
@@ -1021,7 +1011,7 @@ func (cnf *Configurator) DeleteTransportServer(key string) error {
10211011
return fmt.Errorf("error when removing TransportServer %v: %w", key, err)
10221012
}
10231013

1024-
err = cnf.reload(nginx.ReloadForOtherUpdate)
1014+
err = cnf.Reload(nginx.ReloadForOtherUpdate)
10251015
if err != nil {
10261016
return fmt.Errorf("error when removing TransportServer %v: %w", key, err)
10271017
}
@@ -1070,7 +1060,7 @@ func (cnf *Configurator) UpdateEndpoints(ingExes []*IngressEx) error {
10701060
return nil
10711061
}
10721062

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

@@ -1105,7 +1095,7 @@ func (cnf *Configurator) UpdateEndpointsMergeableIngress(mergeableIngresses []*M
11051095
return nil
11061096
}
11071097

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

@@ -1138,7 +1128,7 @@ func (cnf *Configurator) UpdateEndpointsForVirtualServers(virtualServerExes []*V
11381128
return nil
11391129
}
11401130

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

@@ -1185,7 +1175,7 @@ func (cnf *Configurator) UpdateEndpointsForTransportServers(transportServerExes
11851175
nl.Debug(l, "No need to reload nginx")
11861176
return nil
11871177
}
1188-
if err := cnf.reload(nginx.ReloadForEndpointsUpdate); err != nil {
1178+
if err := cnf.Reload(nginx.ReloadForEndpointsUpdate); err != nil {
11891179
return fmt.Errorf("error reloading NGINX when updating endpoints: %w", err)
11901180
}
11911181
return nil
@@ -1272,7 +1262,8 @@ func (cnf *Configurator) DisableReloads() {
12721262
cnf.isReloadsEnabled = false
12731263
}
12741264

1275-
func (cnf *Configurator) reload(isEndpointsUpdate bool) error {
1265+
// Reload reloads nginx if reloads is enabled
1266+
func (cnf *Configurator) Reload(isEndpointsUpdate bool) error {
12761267
if !cnf.isReloadsEnabled {
12771268
return nil
12781269
}
@@ -1398,7 +1389,7 @@ func (cnf *Configurator) UpdateConfig(cfgParams *ConfigParams, resources Extende
13981389
}
13991390

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

@@ -1414,7 +1405,7 @@ func (cnf *Configurator) ReloadForBatchUpdates(batchReloadsEnabled bool) error {
14141405
if !batchReloadsEnabled {
14151406
return nil
14161407
}
1417-
if err := cnf.reload(nginx.ReloadForOtherUpdate); err != nil {
1408+
if err := cnf.Reload(nginx.ReloadForOtherUpdate); err != nil {
14181409
return fmt.Errorf("error when reloading NGINX after a batch event: %w", err)
14191410
}
14201411
return nil
@@ -1439,7 +1430,7 @@ func (cnf *Configurator) UpdateVirtualServers(updatedVSExes []*VirtualServerEx,
14391430
}
14401431
}
14411432

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

@@ -1467,7 +1458,7 @@ func (cnf *Configurator) UpdateTransportServers(updatedTSExes []*TransportServer
14671458
}
14681459
}
14691460

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

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

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

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

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

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

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

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

1840-
err = cnf.reload(nginx.ReloadForOtherUpdate)
1831+
err = cnf.Reload(nginx.ReloadForOtherUpdate)
18411832
if err != nil {
18421833
return warnings, fmt.Errorf("error when updating resources that use Dos: %w", err)
18431834
}
@@ -1925,7 +1916,7 @@ func (cnf *Configurator) RefreshAppProtectUserSigs(
19251916
fmt.Fprintf(&builder, "app_protect_user_defined_signatures %s;\n", fName)
19261917
}
19271918
cnf.nginxManager.CreateAppProtectResourceFile(appProtectUserSigIndex, []byte(builder.String()))
1928-
return allWarnings, cnf.reload(nginx.ReloadForOtherUpdate)
1919+
return allWarnings, cnf.Reload(nginx.ReloadForOtherUpdate)
19291920
}
19301921

19311922
func appProtectDosPolicyFileName(namespace string, name string) string {
@@ -1965,7 +1956,7 @@ func (cnf *Configurator) AddInternalRouteConfig() error {
19651956
return fmt.Errorf("error when writing main Config: %w", err)
19661957
}
19671958
cnf.nginxManager.CreateMainConfig(mainCfgContent)
1968-
if err := cnf.reload(nginx.ReloadForOtherUpdate); err != nil {
1959+
if err := cnf.Reload(nginx.ReloadForOtherUpdate); err != nil {
19691960
return fmt.Errorf("error when reloading nginx: %w", err)
19701961
}
19711962
return nil

‎internal/k8s/controller.go‎

Lines changed: 40 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1781,20 +1781,53 @@ func (lbc *LoadBalancerController) handleSpecialSecretUpdate(secret *api_v1.Secr
17811781
var specialTLSSecretsToUpdate []string
17821782
secretNsName := generateSecretNSName(secret)
17831783

1784+
if ok := lbc.specialSecretValidation(secretNsName, secret, &specialTLSSecretsToUpdate); !ok {
1785+
// if not ok bail early
1786+
return
1787+
}
1788+
1789+
lbc.writeSpecialSecrets(secret, specialTLSSecretsToUpdate)
1790+
1791+
// reload nginx when the TLS special secrets are updated
1792+
switch secretNsName {
1793+
case lbc.specialSecrets.defaultServerSecret, lbc.specialSecrets.wildcardTLSSecret:
1794+
if ok := lbc.performDynamicSSLReload(secret); !ok {
1795+
return
1796+
}
1797+
}
1798+
1799+
lbc.recorder.Eventf(secret, api_v1.EventTypeNormal, "Updated", "the special Secret %v was updated", secretNsName)
1800+
}
1801+
1802+
func (lbc *LoadBalancerController) writeSpecialSecrets(secret *api_v1.Secret, specialTLSSecretsToUpdate []string) {
1803+
lbc.configurator.AddOrUpdateSpecialTLSSecrets(secret, specialTLSSecretsToUpdate)
1804+
}
1805+
1806+
func (lbc *LoadBalancerController) specialSecretValidation(secretNsName string, secret *api_v1.Secret, specialTLSSecretsToUpdate *[]string) bool {
17841807
if secretNsName == lbc.specialSecrets.defaultServerSecret {
1785-
lbc.validationTLSSpecialSecret(secret, configs.DefaultServerSecretFileName, &specialTLSSecretsToUpdate)
1808+
lbc.validationTLSSpecialSecret(secret, configs.DefaultServerSecretFileName, specialTLSSecretsToUpdate)
17861809
}
17871810
if secretNsName == lbc.specialSecrets.wildcardTLSSecret {
1788-
lbc.validationTLSSpecialSecret(secret, configs.WildcardSecretFileName, &specialTLSSecretsToUpdate)
1811+
lbc.validationTLSSpecialSecret(secret, configs.WildcardSecretFileName, specialTLSSecretsToUpdate)
17891812
}
1813+
return true
1814+
}
17901815

1791-
err := lbc.configurator.AddOrUpdateSpecialTLSSecrets(secret, specialTLSSecretsToUpdate)
1792-
if err != nil {
1793-
nl.Errorf(lbc.Logger, "Error when updating the special Secret %v: %v", secretNsName, err)
1816+
func (lbc *LoadBalancerController) performDynamicSSLReload(secret *api_v1.Secret) bool {
1817+
if !lbc.configurator.DynamicSSLReloadEnabled() {
1818+
return lbc.performNGINXReload(secret)
1819+
}
1820+
return true
1821+
}
1822+
1823+
func (lbc *LoadBalancerController) performNGINXReload(secret *api_v1.Secret) bool {
1824+
secretNsName := generateSecretNSName(secret)
1825+
if err := lbc.configurator.Reload(false); err != nil {
1826+
nl.Errorf(lbc.Logger, "error when reloading NGINX when updating the special Secrets: %v", err)
17941827
lbc.recorder.Eventf(secret, api_v1.EventTypeWarning, "UpdatedWithError", "the special Secret %v was updated, but not applied: %v", secretNsName, err)
1795-
return
1828+
return false
17961829
}
1797-
lbc.recorder.Eventf(secret, api_v1.EventTypeNormal, "Updated", "the special Secret %v was updated", secretNsName)
1830+
return true
17981831
}
17991832

18001833
func generateSecretNSName(secret *api_v1.Secret) string {

0 commit comments

Comments
 (0)