From b64293d4dcf6b96985a0a91e8be3f1faa9d218f0 Mon Sep 17 00:00:00 2001 From: Michael Pleshakov Date: Thu, 3 Dec 2020 14:29:27 -0800 Subject: [PATCH] Support warnings for TLS and JWK secrets in Ing --- internal/configs/configurator.go | 121 +++++++++++++++---------- internal/configs/configurator_test.go | 22 +++-- internal/configs/ingress.go | 67 ++++++++++---- internal/configs/ingress_test.go | 122 ++++++++++++++++++++------ internal/configs/warnings.go | 11 ++- internal/k8s/controller.go | 50 +++++++---- 6 files changed, 280 insertions(+), 113 deletions(-) diff --git a/internal/configs/configurator.go b/internal/configs/configurator.go index 45f30193b0..63d9075020 100644 --- a/internal/configs/configurator.go +++ b/internal/configs/configurator.go @@ -230,19 +230,20 @@ func (cnf *Configurator) deleteIngressMetricsLabels(key string) { } // AddOrUpdateIngress adds or updates NGINX configuration for the Ingress resource. -func (cnf *Configurator) AddOrUpdateIngress(ingEx *IngressEx) error { - if err := cnf.addOrUpdateIngress(ingEx); err != nil { - return fmt.Errorf("Error adding or updating ingress %v/%v: %v", ingEx.Ingress.Namespace, ingEx.Ingress.Name, err) +func (cnf *Configurator) AddOrUpdateIngress(ingEx *IngressEx) (Warnings, error) { + warnings, err := cnf.addOrUpdateIngress(ingEx) + if err != nil { + return warnings, fmt.Errorf("Error adding or updating ingress %v/%v: %v", ingEx.Ingress.Namespace, ingEx.Ingress.Name, err) } if err := cnf.nginxManager.Reload(nginx.ReloadForOtherUpdate); err != nil { - return fmt.Errorf("Error reloading NGINX for %v/%v: %v", ingEx.Ingress.Namespace, ingEx.Ingress.Name, err) + return warnings, fmt.Errorf("Error reloading NGINX for %v/%v: %v", ingEx.Ingress.Namespace, ingEx.Ingress.Name, err) } - return nil + return warnings, nil } -func (cnf *Configurator) addOrUpdateIngress(ingEx *IngressEx) error { +func (cnf *Configurator) addOrUpdateIngress(ingEx *IngressEx) (Warnings, error) { apResources := cnf.updateApResources(ingEx) if jwtKey, exists := ingEx.Ingress.Annotations[JWTKeyAnnotation]; exists { @@ -253,12 +254,12 @@ func (cnf *Configurator) addOrUpdateIngress(ingEx *IngressEx) error { } isMinion := false - nginxCfg := generateNginxCfg(ingEx, apResources, isMinion, cnf.cfgParams, cnf.isPlus, cnf.IsResolverConfigured(), + nginxCfg, warnings := generateNginxCfg(ingEx, apResources, isMinion, cnf.cfgParams, cnf.isPlus, cnf.IsResolverConfigured(), cnf.staticCfgParams, cnf.isWildcardEnabled) name := objectMetaToFileName(&ingEx.Ingress.ObjectMeta) content, err := cnf.templateExecutor.ExecuteIngressConfigTemplate(&nginxCfg) if err != nil { - return fmt.Errorf("Error generating Ingress Config %v: %v", name, err) + return warnings, fmt.Errorf("Error generating Ingress Config %v: %v", name, err) } cnf.nginxManager.CreateConfig(name, content) @@ -266,41 +267,45 @@ func (cnf *Configurator) addOrUpdateIngress(ingEx *IngressEx) error { if (cnf.isPlus && cnf.isPrometheusEnabled) || cnf.isLatencyMetricsEnabled { cnf.updateIngressMetricsLabels(ingEx, nginxCfg.Upstreams) } - return nil + return warnings, nil } // AddOrUpdateMergeableIngress adds or updates NGINX configuration for the Ingress resources with Mergeable Types. -func (cnf *Configurator) AddOrUpdateMergeableIngress(mergeableIngs *MergeableIngresses) error { - if err := cnf.addOrUpdateMergeableIngress(mergeableIngs); err != nil { - return fmt.Errorf("Error when adding or updating ingress %v/%v: %v", mergeableIngs.Master.Ingress.Namespace, mergeableIngs.Master.Ingress.Name, err) +func (cnf *Configurator) AddOrUpdateMergeableIngress(mergeableIngs *MergeableIngresses) (Warnings, error) { + warnings, err := cnf.addOrUpdateMergeableIngress(mergeableIngs) + if err != nil { + return warnings, fmt.Errorf("Error when adding or updating ingress %v/%v: %v", mergeableIngs.Master.Ingress.Namespace, mergeableIngs.Master.Ingress.Name, err) } if err := cnf.nginxManager.Reload(nginx.ReloadForOtherUpdate); err != nil { - return fmt.Errorf("Error reloading NGINX for %v/%v: %v", mergeableIngs.Master.Ingress.Namespace, mergeableIngs.Master.Ingress.Name, err) + return warnings, fmt.Errorf("Error reloading NGINX for %v/%v: %v", mergeableIngs.Master.Ingress.Namespace, mergeableIngs.Master.Ingress.Name, err) } - return nil + return warnings, nil } -func (cnf *Configurator) addOrUpdateMergeableIngress(mergeableIngs *MergeableIngresses) error { +func (cnf *Configurator) addOrUpdateMergeableIngress(mergeableIngs *MergeableIngresses) (Warnings, error) { masterApResources := cnf.updateApResources(mergeableIngs.Master) + // LocalSecretStore will not set Path if the secret is not on the filesystem. + // However, NGINX configuration for an Ingress resource, to handle the case of a missing secret, + // relies on the path to be always configured. + if jwtKey, exists := mergeableIngs.Master.Ingress.Annotations[JWTKeyAnnotation]; exists { + mergeableIngs.Master.SecretRefs[jwtKey].Path = cnf.nginxManager.GetFilenameForSecret(mergeableIngs.Master.Ingress.Namespace + "-" + jwtKey) + } for _, minion := range mergeableIngs.Minions { if jwtKey, exists := minion.Ingress.Annotations[JWTKeyAnnotation]; exists { - // LocalSecretStore will not set Path if the secret is not on the filesystem. - // However, NGINX configuration for an Ingress resource, to handle the case of a missing secret, - // relies on the path to be always configured. minion.SecretRefs[jwtKey].Path = cnf.nginxManager.GetFilenameForSecret(minion.Ingress.Namespace + "-" + jwtKey) } } - nginxCfg := generateNginxCfgForMergeableIngresses(mergeableIngs, masterApResources, cnf.cfgParams, cnf.isPlus, + nginxCfg, warnings := generateNginxCfgForMergeableIngresses(mergeableIngs, masterApResources, cnf.cfgParams, cnf.isPlus, cnf.IsResolverConfigured(), cnf.staticCfgParams, cnf.isWildcardEnabled) name := objectMetaToFileName(&mergeableIngs.Master.Ingress.ObjectMeta) content, err := cnf.templateExecutor.ExecuteIngressConfigTemplate(&nginxCfg) if err != nil { - return fmt.Errorf("Error generating Ingress Config %v: %v", name, err) + return warnings, fmt.Errorf("Error generating Ingress Config %v: %v", name, err) } cnf.nginxManager.CreateConfig(name, content) @@ -314,7 +319,7 @@ func (cnf *Configurator) addOrUpdateMergeableIngress(mergeableIngs *MergeableIng cnf.updateIngressMetricsLabels(mergeableIngs.Master, nginxCfg.Upstreams) } - return nil + return warnings, nil } func (cnf *Configurator) updateVirtualServerMetricsLabels(virtualServerEx *VirtualServerEx, upstreams []version2.Upstream) { @@ -565,17 +570,19 @@ func (cnf *Configurator) AddOrUpdateResources(ingExes []*IngressEx, mergeableIng allWarnings := newWarnings() for _, ingEx := range ingExes { - err := cnf.addOrUpdateIngress(ingEx) + warnings, err := cnf.addOrUpdateIngress(ingEx) if err != nil { return allWarnings, fmt.Errorf("Error adding or updating ingress %v/%v: %v", ingEx.Ingress.Namespace, ingEx.Ingress.Name, err) } + allWarnings.Add(warnings) } for _, m := range mergeableIngresses { - err := cnf.addOrUpdateMergeableIngress(m) + warnings, err := cnf.addOrUpdateMergeableIngress(m) if err != nil { return allWarnings, fmt.Errorf("Error adding or updating mergeableIngress %v/%v: %v", m.Master.Ingress.Namespace, m.Master.Ingress.Name, err) } + allWarnings.Add(warnings) } for _, vsEx := range virtualServerExes { @@ -704,7 +711,8 @@ func (cnf *Configurator) UpdateEndpoints(ingExes []*IngressEx) error { reloadPlus := false for _, ingEx := range ingExes { - err := cnf.addOrUpdateIngress(ingEx) + // It is safe to ignore warnings here as no new warnings should appear when updating Endpoints for Ingresses + _, err := cnf.addOrUpdateIngress(ingEx) if err != nil { return fmt.Errorf("Error adding or updating ingress %v/%v: %v", ingEx.Ingress.Namespace, ingEx.Ingress.Name, err) } @@ -735,7 +743,8 @@ func (cnf *Configurator) UpdateEndpointsMergeableIngress(mergeableIngresses []*M reloadPlus := false for i := range mergeableIngresses { - err := cnf.addOrUpdateMergeableIngress(mergeableIngresses[i]) + // It is safe to ignore warnings here as no new warnings should appear when updating Endpoints for Ingresses + _, err := cnf.addOrUpdateMergeableIngress(mergeableIngresses[i]) if err != nil { return fmt.Errorf("Error adding or updating mergeableIngress %v/%v: %v", mergeableIngresses[i].Master.Ingress.Namespace, mergeableIngresses[i].Master.Ingress.Name, err) } @@ -953,14 +962,18 @@ func (cnf *Configurator) UpdateConfig(cfgParams *ConfigParams, ingExes []*Ingres cnf.nginxManager.CreateMainConfig(mainCfgContent) for _, ingEx := range ingExes { - if err := cnf.addOrUpdateIngress(ingEx); err != nil { + warnings, err := cnf.addOrUpdateIngress(ingEx) + if err != nil { return allWarnings, err } + allWarnings.Add(warnings) } for _, mergeableIng := range mergeableIngs { - if err := cnf.addOrUpdateMergeableIngress(mergeableIng); err != nil { + warnings, err := cnf.addOrUpdateMergeableIngress(mergeableIng) + if err != nil { return allWarnings, err } + allWarnings.Add(warnings) } for _, vsEx := range virtualServerExes { warnings, err := cnf.addOrUpdateVirtualServer(vsEx) @@ -1188,80 +1201,92 @@ func generateApResourceFileContent(apResource *unstructured.Unstructured) []byte } // AddOrUpdateAppProtectResource updates Ingresses that use App Protect Resources -func (cnf *Configurator) AddOrUpdateAppProtectResource(resource *unstructured.Unstructured, ingExes []*IngressEx, mergeableIngresses []*MergeableIngresses) error { +func (cnf *Configurator) AddOrUpdateAppProtectResource(resource *unstructured.Unstructured, ingExes []*IngressEx, mergeableIngresses []*MergeableIngresses) (Warnings, error) { + allWarnings := newWarnings() + for _, ingEx := range ingExes { - err := cnf.addOrUpdateIngress(ingEx) + warnings, err := cnf.addOrUpdateIngress(ingEx) if err != nil { - return fmt.Errorf("Error adding or updating ingress %v/%v: %v", ingEx.Ingress.Namespace, ingEx.Ingress.Name, err) + return allWarnings, fmt.Errorf("Error adding or updating ingress %v/%v: %v", ingEx.Ingress.Namespace, ingEx.Ingress.Name, err) } + allWarnings.Add(warnings) } for _, m := range mergeableIngresses { - err := cnf.addOrUpdateMergeableIngress(m) + warnings, err := cnf.addOrUpdateMergeableIngress(m) if err != nil { - return fmt.Errorf("Error adding or updating mergeableIngress %v/%v: %v", m.Master.Ingress.Namespace, m.Master.Ingress.Name, err) + return allWarnings, fmt.Errorf("Error adding or updating mergeableIngress %v/%v: %v", m.Master.Ingress.Namespace, m.Master.Ingress.Name, err) } + allWarnings.Add(warnings) } if err := cnf.nginxManager.Reload(nginx.ReloadForOtherUpdate); err != nil { - return fmt.Errorf("Error when reloading NGINX when updating %v: %v", resource.GetKind(), err) + return allWarnings, fmt.Errorf("Error when reloading NGINX when updating %v: %v", resource.GetKind(), err) } - return nil + return allWarnings, nil } // DeleteAppProtectPolicy updates Ingresses that use AP Policy after that policy is deleted -func (cnf *Configurator) DeleteAppProtectPolicy(polNamespaceName string, ingExes []*IngressEx, mergeableIngresses []*MergeableIngresses) error { +func (cnf *Configurator) DeleteAppProtectPolicy(polNamespaceName string, ingExes []*IngressEx, mergeableIngresses []*MergeableIngresses) (Warnings, error) { fName := strings.Replace(polNamespaceName, "/", "_", 1) polFileName := appProtectPolicyFolder + fName cnf.nginxManager.DeleteAppProtectResourceFile(polFileName) + allWarnings := newWarnings() + for _, ingEx := range ingExes { - err := cnf.addOrUpdateIngress(ingEx) + warnings, err := cnf.addOrUpdateIngress(ingEx) if err != nil { - return fmt.Errorf("Error adding or updating ingress %v/%v: %v", ingEx.Ingress.Namespace, ingEx.Ingress.Name, err) + return allWarnings, fmt.Errorf("Error adding or updating ingress %v/%v: %v", ingEx.Ingress.Namespace, ingEx.Ingress.Name, err) } + allWarnings.Add(warnings) } for _, m := range mergeableIngresses { - err := cnf.addOrUpdateMergeableIngress(m) + warnings, err := cnf.addOrUpdateMergeableIngress(m) if err != nil { - return fmt.Errorf("Error adding or updating mergeableIngress %v/%v: %v", m.Master.Ingress.Namespace, m.Master.Ingress.Name, err) + return allWarnings, fmt.Errorf("Error adding or updating mergeableIngress %v/%v: %v", m.Master.Ingress.Namespace, m.Master.Ingress.Name, err) } + allWarnings.Add(warnings) } if err := cnf.nginxManager.Reload(nginx.ReloadForOtherUpdate); err != nil { - return fmt.Errorf("Error when reloading NGINX when removing App Protect Policy: %v", err) + return allWarnings, fmt.Errorf("Error when reloading NGINX when removing App Protect Policy: %v", err) } - return nil + return allWarnings, nil } // DeleteAppProtectLogConf updates Ingresses that use AP Log Configuration after that policy is deleted -func (cnf *Configurator) DeleteAppProtectLogConf(logConfNamespaceName string, ingExes []*IngressEx, mergeableIngresses []*MergeableIngresses) error { +func (cnf *Configurator) DeleteAppProtectLogConf(logConfNamespaceName string, ingExes []*IngressEx, mergeableIngresses []*MergeableIngresses) (Warnings, error) { fName := strings.Replace(logConfNamespaceName, "/", "_", 1) logConfFileName := appProtectLogConfFolder + fName cnf.nginxManager.DeleteAppProtectResourceFile(logConfFileName) + allWarnings := newWarnings() + for _, ingEx := range ingExes { - err := cnf.addOrUpdateIngress(ingEx) + warnings, err := cnf.addOrUpdateIngress(ingEx) if err != nil { - return fmt.Errorf("Error adding or updating ingress %v/%v: %v", ingEx.Ingress.Namespace, ingEx.Ingress.Name, err) + return allWarnings, fmt.Errorf("Error adding or updating ingress %v/%v: %v", ingEx.Ingress.Namespace, ingEx.Ingress.Name, err) } + allWarnings.Add(warnings) } for _, m := range mergeableIngresses { - err := cnf.addOrUpdateMergeableIngress(m) + warnings, err := cnf.addOrUpdateMergeableIngress(m) if err != nil { - return fmt.Errorf("Error adding or updating mergeableIngress %v/%v: %v", m.Master.Ingress.Namespace, m.Master.Ingress.Name, err) + return allWarnings, fmt.Errorf("Error adding or updating mergeableIngress %v/%v: %v", m.Master.Ingress.Namespace, m.Master.Ingress.Name, err) } + allWarnings.Add(warnings) } if err := cnf.nginxManager.Reload(nginx.ReloadForOtherUpdate); err != nil { - return fmt.Errorf("Error when reloading NGINX when removing App Protect Log Configuration: %v", err) + return allWarnings, fmt.Errorf("Error when reloading NGINX when removing App Protect Log Configuration: %v", err) } - return nil + return allWarnings, nil } // AddInternalRouteConfig adds internal route server to NGINX Configuration and diff --git a/internal/configs/configurator_test.go b/internal/configs/configurator_test.go index 4235a89037..6f42527bc7 100644 --- a/internal/configs/configurator_test.go +++ b/internal/configs/configurator_test.go @@ -67,10 +67,13 @@ func TestAddOrUpdateIngress(t *testing.T) { ingress := createCafeIngressEx() - err = cnf.AddOrUpdateIngress(&ingress) + warnings, err := cnf.AddOrUpdateIngress(&ingress) if err != nil { t.Errorf("AddOrUpdateIngress returned: \n%v, but expected: \n%v", err, nil) } + if len(warnings) != 0 { + t.Errorf("AddOrUpdateIngress returned warnings: %v", warnings) + } cnfHasIngress := cnf.HasIngress(ingress.Ingress) if !cnfHasIngress { @@ -86,10 +89,13 @@ func TestAddOrUpdateMergeableIngress(t *testing.T) { mergeableIngess := createMergeableCafeIngress() - err = cnf.AddOrUpdateMergeableIngress(mergeableIngess) + warnings, err := cnf.AddOrUpdateMergeableIngress(mergeableIngess) if err != nil { t.Errorf("AddOrUpdateMergeableIngress returned \n%v, expected \n%v", err, nil) } + if len(warnings) != 0 { + t.Errorf("AddOrUpdateMergeableIngress returned warnings: %v", warnings) + } cnfHasMergeableIngress := cnf.HasIngress(mergeableIngess.Master.Ingress) if !cnfHasMergeableIngress { @@ -105,9 +111,12 @@ func TestAddOrUpdateIngressFailsWithInvalidIngressTemplate(t *testing.T) { ingress := createCafeIngressEx() - err = cnf.AddOrUpdateIngress(&ingress) + warnings, err := cnf.AddOrUpdateIngress(&ingress) if err == nil { - t.Errorf("AddOrUpdateIngressFailsWithInvalidTemplate returned \n%v, but expected \n%v", nil, "template execution error") + t.Errorf("AddOrUpdateIngress returned \n%v, but expected \n%v", nil, "template execution error") + } + if len(warnings) != 0 { + t.Errorf("AddOrUpdateIngress returned warnings: %v", warnings) } } @@ -119,10 +128,13 @@ func TestAddOrUpdateMergeableIngressFailsWithInvalidIngressTemplate(t *testing.T mergeableIngess := createMergeableCafeIngress() - err = cnf.AddOrUpdateMergeableIngress(mergeableIngess) + warnings, err := cnf.AddOrUpdateMergeableIngress(mergeableIngess) if err == nil { t.Errorf("AddOrUpdateMergeableIngress returned \n%v, but expected \n%v", nil, "template execution error") } + if len(warnings) != 0 { + t.Errorf("AddOrUpdateMergeableIngress returned warnings: %v", warnings) + } } func TestUpdateEndpoints(t *testing.T) { diff --git a/internal/configs/ingress.go b/internal/configs/ingress.go index 2b718b1ba4..04c7959bcd 100644 --- a/internal/configs/ingress.go +++ b/internal/configs/ingress.go @@ -9,6 +9,7 @@ import ( "github.com/nginxinc/kubernetes-ingress/internal/k8s/secrets" api_v1 "k8s.io/api/core/v1" networking "k8s.io/api/networking/v1beta1" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" @@ -55,7 +56,7 @@ type MergeableIngresses struct { } func generateNginxCfg(ingEx *IngressEx, apResources map[string]string, isMinion bool, baseCfgParams *ConfigParams, isPlus bool, - isResolverConfigured bool, staticParams *StaticConfigParams, isWildcardEnabled bool) version1.IngressNginxConfig { + isResolverConfigured bool, staticParams *StaticConfigParams, isWildcardEnabled bool) (version1.IngressNginxConfig, Warnings) { hasAppProtect := staticParams.MainAppProtectLoadModule cfgParams := parseAnnotations(ingEx, baseCfgParams, isPlus, hasAppProtect, staticParams.EnableInternalRoutes) @@ -87,6 +88,8 @@ func generateNginxCfg(ingEx *IngressEx, apResources map[string]string, isMinion } } + allWarnings := newWarnings() + var servers []version1.Server for _, rule := range ingEx.Ingress.Spec.Rules { @@ -132,7 +135,8 @@ func generateNginxCfg(ingEx *IngressEx, apResources map[string]string, isMinion SpiffeCerts: cfgParams.SpiffeServerCerts, } - addSSLConfig(&server, rule.Host, ingEx.Ingress.Spec.TLS, ingEx.SecretRefs, isWildcardEnabled) + warnings := addSSLConfig(&server, ingEx.Ingress, rule.Host, ingEx.Ingress.Spec.TLS, ingEx.SecretRefs, isWildcardEnabled) + allWarnings.Add(warnings) if hasAppProtect { server.AppProtectPolicy = apResources[appProtectPolicyKey] @@ -140,11 +144,12 @@ func generateNginxCfg(ingEx *IngressEx, apResources map[string]string, isMinion } if !isMinion && cfgParams.JWTKey != "" { - jwtAuth, redirectLoc := generateJWTConfig(ingEx.SecretRefs, &cfgParams, getNameForRedirectLocation(ingEx.Ingress)) + jwtAuth, redirectLoc, warnings := generateJWTConfig(ingEx.Ingress, ingEx.SecretRefs, &cfgParams, getNameForRedirectLocation(ingEx.Ingress)) server.JWTAuth = jwtAuth if redirectLoc != nil { server.JWTRedirectLocations = append(server.JWTRedirectLocations, *redirectLoc) } + allWarnings.Add(warnings) } var locations []version1.Location @@ -189,11 +194,12 @@ func generateNginxCfg(ingEx *IngressEx, apResources map[string]string, isMinion ssl, grpcServices[path.Backend.ServiceName], proxySSLName, path.PathType, path.Backend.ServiceName) if isMinion && cfgParams.JWTKey != "" { - jwtAuth, redirectLoc := generateJWTConfig(ingEx.SecretRefs, &cfgParams, getNameForRedirectLocation(ingEx.Ingress)) + jwtAuth, redirectLoc, warnings := generateJWTConfig(ingEx.Ingress, ingEx.SecretRefs, &cfgParams, getNameForRedirectLocation(ingEx.Ingress)) loc.JWTAuth = jwtAuth if redirectLoc != nil { server.JWTRedirectLocations = append(server.JWTRedirectLocations, *redirectLoc) } + allWarnings.Add(warnings) } locations = append(locations, loc) @@ -246,16 +252,19 @@ func generateNginxCfg(ingEx *IngressEx, apResources map[string]string, isMinion Annotations: ingEx.Ingress.Annotations, }, SpiffeClientCerts: staticParams.NginxServiceMesh && !cfgParams.SpiffeServerCerts, - } + }, allWarnings } -func generateJWTConfig(secretRefs map[string]*secrets.SecretReference, cfgParams *ConfigParams, redirectLocationName string) (*version1.JWTAuth, *version1.JWTRedirectLocation) { +func generateJWTConfig(owner runtime.Object, secretRefs map[string]*secrets.SecretReference, cfgParams *ConfigParams, + redirectLocationName string) (*version1.JWTAuth, *version1.JWTRedirectLocation, Warnings) { + warnings := newWarnings() + secret := secretRefs[cfgParams.JWTKey] if secret.Error != nil { - // TO-DO: add a warning + warnings.AddWarningf(owner, "JWK secret %s is invalid: %v", cfgParams.JWTKey, secret.Error) } else if secret.Type != secrets.SecretTypeJWK { - // TO-DO: add a warning + warnings.AddWarningf(owner, "JWK secret %s is of a wrong type '%s', must be '%s'", cfgParams.JWTKey, secret.Type, secrets.SecretTypeJWK) } // Key is configured for all cases, including when the secret is (1) invalid or (2) of a wrong type. @@ -276,10 +285,13 @@ func generateJWTConfig(secretRefs map[string]*secrets.SecretReference, cfgParams } } - return jwtAuth, redirectLocation + return jwtAuth, redirectLocation, warnings } -func addSSLConfig(server *version1.Server, host string, ingressTLS []networking.IngressTLS, secretRefs map[string]*secrets.SecretReference, isWildcardEnabled bool) { +func addSSLConfig(server *version1.Server, owner runtime.Object, host string, ingressTLS []networking.IngressTLS, + secretRefs map[string]*secrets.SecretReference, isWildcardEnabled bool) Warnings { + warnings := newWarnings() + var tlsEnabled bool var tlsSecret string @@ -294,7 +306,7 @@ func addSSLConfig(server *version1.Server, host string, ingressTLS []networking. } if !tlsEnabled { - return + return warnings } var pemFile string @@ -303,10 +315,10 @@ func addSSLConfig(server *version1.Server, host string, ingressTLS []networking. secret := secretRefs[tlsSecret] if secret.Error != nil { pemFile = pemFileNameForMissingTLSSecret - // TO-DO: add a warning + warnings.AddWarningf(owner, "TLS secret %s is invalid: %v", tlsSecret, secret.Error) } else if secret.Type != api_v1.SecretTypeTLS { pemFile = pemFileNameForMissingTLSSecret - // TO-DO: add a warning + warnings.AddWarningf(owner, "TLS secret %s is of a wrong type '%s', must be '%s'", tlsSecret, secret.Type, api_v1.SecretTypeTLS) } else { pemFile = secret.Path } @@ -314,7 +326,7 @@ func addSSLConfig(server *version1.Server, host string, ingressTLS []networking. pemFile = pemFileNameForWildcardTLSSecret } else { pemFile = pemFileNameForMissingTLSSecret - // TO-DO: add a warning + warnings.AddWarningf(owner, "TLS termination for host '%s' requires specifying a TLS secret or configuring a global wildcard TLS secret", host) } server.SSL = true @@ -323,6 +335,8 @@ func addSSLConfig(server *version1.Server, host string, ingressTLS []networking. if pemFile == pemFileNameForMissingTLSSecret { server.SSLCiphers = "NULL" } + + return warnings } func generateIngressPath(path string, pathType *networking.PathType) string { @@ -482,7 +496,7 @@ func upstreamMapToSlice(upstreams map[string]version1.Upstream) []version1.Upstr func generateNginxCfgForMergeableIngresses(mergeableIngs *MergeableIngresses, masterApResources map[string]string, baseCfgParams *ConfigParams, isPlus bool, isResolverConfigured bool, staticParams *StaticConfigParams, - isWildcardEnabled bool) version1.IngressNginxConfig { + isWildcardEnabled bool) (version1.IngressNginxConfig, Warnings) { var masterServer version1.Server var locations []version1.Location @@ -491,6 +505,7 @@ func generateNginxCfgForMergeableIngresses(mergeableIngs *MergeableIngresses, ma var keepalive string // replace master with a deepcopy because we will modify it + originalMaster := mergeableIngs.Master.Ingress mergeableIngs.Master.Ingress = mergeableIngs.Master.Ingress.DeepCopy() removedAnnotations := filterMasterAnnotations(mergeableIngs.Master.Ingress.Annotations) @@ -500,7 +515,14 @@ func generateNginxCfgForMergeableIngresses(mergeableIngs *MergeableIngresses, ma } isMinion := false - masterNginxCfg := generateNginxCfg(mergeableIngs.Master, masterApResources, isMinion, baseCfgParams, isPlus, isResolverConfigured, staticParams, isWildcardEnabled) + masterNginxCfg, warnings := generateNginxCfg(mergeableIngs.Master, masterApResources, isMinion, baseCfgParams, isPlus, isResolverConfigured, staticParams, isWildcardEnabled) + + // because mergeableIngs.Master.Ingress is a deepcopy of the original master + // we need to change the key in the warnings to the original master + if _, exists := warnings[mergeableIngs.Master.Ingress]; exists { + warnings[originalMaster] = warnings[mergeableIngs.Master.Ingress] + delete(warnings, mergeableIngs.Master.Ingress) + } masterServer = masterNginxCfg.Servers[0] masterServer.Locations = []version1.Location{} @@ -514,6 +536,7 @@ func generateNginxCfgForMergeableIngresses(mergeableIngs *MergeableIngresses, ma minions := mergeableIngs.Minions for _, minion := range minions { // replace minion with a deepcopy because we will modify it + originalMinion := minion.Ingress minion.Ingress = minion.Ingress.DeepCopy() // Remove the default backend so that "/" will not be generated @@ -531,7 +554,15 @@ func generateNginxCfgForMergeableIngresses(mergeableIngs *MergeableIngresses, ma isMinion := true // App Protect Resources not allowed in minions - pass empty map dummyApResources := make(map[string]string) - nginxCfg := generateNginxCfg(minion, dummyApResources, isMinion, baseCfgParams, isPlus, isResolverConfigured, staticParams, isWildcardEnabled) + nginxCfg, minionWarnings := generateNginxCfg(minion, dummyApResources, isMinion, baseCfgParams, isPlus, isResolverConfigured, staticParams, isWildcardEnabled) + warnings.Add(minionWarnings) + + // because minion.Ingress is a deepcopy of the original minion + // we need to change the key in the warnings to the original minion + if _, exists := warnings[minion.Ingress]; exists { + warnings[originalMinion] = warnings[minion.Ingress] + delete(warnings, minion.Ingress) + } for _, server := range nginxCfg.Servers { for _, loc := range server.Locations { @@ -556,7 +587,7 @@ func generateNginxCfgForMergeableIngresses(mergeableIngs *MergeableIngresses, ma Keepalive: keepalive, Ingress: masterNginxCfg.Ingress, SpiffeClientCerts: staticParams.NginxServiceMesh && !baseCfgParams.SpiffeServerCerts, - } + }, warnings } func isSSLEnabled(isSSLService bool, cfgParams ConfigParams, staticCfgParams *StaticConfigParams) bool { diff --git a/internal/configs/ingress_test.go b/internal/configs/ingress_test.go index 816c1e03f1..9f758a648b 100644 --- a/internal/configs/ingress_test.go +++ b/internal/configs/ingress_test.go @@ -23,11 +23,14 @@ func TestGenerateNginxCfg(t *testing.T) { expected := createExpectedConfigForCafeIngressEx() apRes := make(map[string]string) - result := generateNginxCfg(&cafeIngressEx, apRes, false, configParams, false, false, &StaticConfigParams{}, false) + result, warnings := generateNginxCfg(&cafeIngressEx, apRes, false, configParams, false, false, &StaticConfigParams{}, false) if diff := cmp.Diff(expected, result); diff != "" { t.Errorf("generateNginxCfg() returned unexpected result (-want +got):\n%s", diff) } + if len(warnings) != 0 { + t.Errorf("generateNginxCfg() returned warnings: %v", warnings) + } } func TestGenerateNginxCfgForJWT(t *testing.T) { @@ -58,7 +61,7 @@ func TestGenerateNginxCfgForJWT(t *testing.T) { } apRes := make(map[string]string) - result := generateNginxCfg(&cafeIngressEx, apRes, false, configParams, true, false, &StaticConfigParams{}, false) + result, warnings := generateNginxCfg(&cafeIngressEx, apRes, false, configParams, true, false, &StaticConfigParams{}, false) if !reflect.DeepEqual(result.Servers[0].JWTAuth, expected.Servers[0].JWTAuth) { t.Errorf("generateNginxCfg returned \n%v, but expected \n%v", result.Servers[0].JWTAuth, expected.Servers[0].JWTAuth) @@ -66,6 +69,9 @@ func TestGenerateNginxCfgForJWT(t *testing.T) { if !reflect.DeepEqual(result.Servers[0].JWTRedirectLocations, expected.Servers[0].JWTRedirectLocations) { t.Errorf("generateNginxCfg returned \n%v, but expected \n%v", result.Servers[0].JWTRedirectLocations, expected.Servers[0].JWTRedirectLocations) } + if len(warnings) != 0 { + t.Errorf("generateNginxCfg returned warnings: %v", warnings) + } } func TestGenerateNginxCfgWithMissingTLSSecret(t *testing.T) { @@ -74,13 +80,22 @@ func TestGenerateNginxCfgWithMissingTLSSecret(t *testing.T) { configParams := NewDefaultConfigParams() apRes := make(map[string]string) - result := generateNginxCfg(&cafeIngressEx, apRes, false, configParams, false, false, &StaticConfigParams{}, false) + result, resultWarnings := generateNginxCfg(&cafeIngressEx, apRes, false, configParams, false, false, &StaticConfigParams{}, false) expectedCiphers := "NULL" + expectedWarnings := Warnings{ + cafeIngressEx.Ingress: { + "TLS secret cafe-secret is invalid: secret doesn't exist", + }, + } + resultCiphers := result.Servers[0].SSLCiphers if !reflect.DeepEqual(resultCiphers, expectedCiphers) { t.Errorf("generateNginxCfg returned SSLCiphers %v, but expected %v", resultCiphers, expectedCiphers) } + if diff := cmp.Diff(expectedWarnings, resultWarnings); diff != "" { + t.Errorf("generateNginxCfg returned unexpected result (-want +got):\n%s", diff) + } } func TestGenerateNginxCfgWithWildcardTLSSecret(t *testing.T) { @@ -89,7 +104,7 @@ func TestGenerateNginxCfgWithWildcardTLSSecret(t *testing.T) { configParams := NewDefaultConfigParams() apRes := make(map[string]string) - result := generateNginxCfg(&cafeIngressEx, apRes, false, configParams, false, false, &StaticConfigParams{}, true) + result, warnings := generateNginxCfg(&cafeIngressEx, apRes, false, configParams, false, false, &StaticConfigParams{}, true) resultServer := result.Servers[0] if !reflect.DeepEqual(resultServer.SSLCertificate, pemFileNameForWildcardTLSSecret) { @@ -98,6 +113,9 @@ func TestGenerateNginxCfgWithWildcardTLSSecret(t *testing.T) { if !reflect.DeepEqual(resultServer.SSLCertificateKey, pemFileNameForWildcardTLSSecret) { t.Errorf("generateNginxCfg returned SSLCertificateKey %v, but expected %v", resultServer.SSLCertificateKey, pemFileNameForWildcardTLSSecret) } + if len(warnings) != 0 { + t.Errorf("generateNginxCfg returned warnings: %v", warnings) + } } func TestPathOrDefaultReturnDefault(t *testing.T) { @@ -307,11 +325,14 @@ func TestGenerateNginxCfgForMergeableIngresses(t *testing.T) { configParams := NewDefaultConfigParams() masterApRes := make(map[string]string) - result := generateNginxCfgForMergeableIngresses(mergeableIngresses, masterApRes, configParams, false, false, &StaticConfigParams{}, false) + result, warnings := generateNginxCfgForMergeableIngresses(mergeableIngresses, masterApRes, configParams, false, false, &StaticConfigParams{}, false) if diff := cmp.Diff(expected, result); diff != "" { t.Errorf("generateNginxCfgForMergeableIngresses() returned unexpected result (-want +got):\n%s", diff) } + if len(warnings) != 0 { + t.Errorf("generateNginxCfgForMergeableIngresses() returned warnings: %v", warnings) + } } func TestGenerateNginxConfigForCrossNamespaceMergeableIngresses(t *testing.T) { @@ -329,11 +350,14 @@ func TestGenerateNginxConfigForCrossNamespaceMergeableIngresses(t *testing.T) { configParams := NewDefaultConfigParams() emptyApResources := make(map[string]string) - result := generateNginxCfgForMergeableIngresses(mergeableIngresses, emptyApResources, configParams, false, false, &StaticConfigParams{}, false) + result, warnings := generateNginxCfgForMergeableIngresses(mergeableIngresses, emptyApResources, configParams, false, false, &StaticConfigParams{}, false) if diff := cmp.Diff(expected, result); diff != "" { t.Errorf("generateNginxCfgForMergeableIngresses() returned unexpected result (-want +got):\n%s", diff) } + if len(warnings) != 0 { + t.Errorf("generateNginxCfgForMergeableIngresses() returned warnings: %v", warnings) + } } func TestGenerateNginxCfgForMergeableIngressesForJWT(t *testing.T) { @@ -386,7 +410,7 @@ func TestGenerateNginxCfgForMergeableIngressesForJWT(t *testing.T) { isPlus := true masterApRes := make(map[string]string) - result := generateNginxCfgForMergeableIngresses(mergeableIngresses, masterApRes, configParams, isPlus, false, &StaticConfigParams{}, false) + result, warnings := generateNginxCfgForMergeableIngresses(mergeableIngresses, masterApRes, configParams, isPlus, false, &StaticConfigParams{}, false) if !reflect.DeepEqual(result.Servers[0].JWTAuth, expected.Servers[0].JWTAuth) { t.Errorf("generateNginxCfgForMergeableIngresses returned \n%v, but expected \n%v", result.Servers[0].JWTAuth, expected.Servers[0].JWTAuth) @@ -397,6 +421,9 @@ func TestGenerateNginxCfgForMergeableIngressesForJWT(t *testing.T) { if !reflect.DeepEqual(result.Servers[0].JWTRedirectLocations, expected.Servers[0].JWTRedirectLocations) { t.Errorf("generateNginxCfgForMergeableIngresses returned \n%v, but expected \n%v", result.Servers[0].JWTRedirectLocations, expected.Servers[0].JWTRedirectLocations) } + if len(warnings) != 0 { + t.Errorf("generateNginxCfgForMergeableIngresses returned warnings: %v", warnings) + } } func createMergeableCafeIngress() *MergeableIngresses { @@ -756,12 +783,15 @@ func TestGenerateNginxCfgForSpiffe(t *testing.T) { } apResources := make(map[string]string) - result := generateNginxCfg(&cafeIngressEx, apResources, false, configParams, false, false, + result, warnings := generateNginxCfg(&cafeIngressEx, apResources, false, configParams, false, false, &StaticConfigParams{NginxServiceMesh: true}, false) if diff := cmp.Diff(expected, result); diff != "" { t.Errorf("generateNginxCfg() returned unexpected result (-want +got):\n%s", diff) } + if len(warnings) != 0 { + t.Errorf("generateNginxCfg() returned warnings: %v", warnings) + } } func TestGenerateNginxCfgForInternalRoute(t *testing.T) { @@ -775,12 +805,15 @@ func TestGenerateNginxCfgForInternalRoute(t *testing.T) { expected.Ingress.Annotations[internalRouteAnnotation] = "true" apResources := make(map[string]string) - result := generateNginxCfg(&cafeIngressEx, apResources, false, configParams, false, false, + result, warnings := generateNginxCfg(&cafeIngressEx, apResources, false, configParams, false, false, &StaticConfigParams{NginxServiceMesh: true, EnableInternalRoutes: true}, false) if diff := cmp.Diff(expected, result); diff != "" { t.Errorf("generateNginxCfg() returned unexpected result (-want +got):\n%s", diff) } + if len(warnings) != 0 { + t.Errorf("generateNginxCfg() returned warnings: %v", warnings) + } } func TestIsSSLEnabled(t *testing.T) { @@ -854,7 +887,8 @@ func TestAddSSLConfig(t *testing.T) { tls []networking.IngressTLS secretRefs map[string]*secrets.SecretReference isWildcardEnabled bool - expected version1.Server + expectedServer version1.Server + expectedWarnings Warnings msg string }{ { @@ -872,7 +906,8 @@ func TestAddSSLConfig(t *testing.T) { }, }, isWildcardEnabled: false, - expected: version1.Server{}, + expectedServer: version1.Server{}, + expectedWarnings: Warnings{}, msg: "TLS termination for different host", }, { @@ -890,12 +925,13 @@ func TestAddSSLConfig(t *testing.T) { }, }, isWildcardEnabled: false, - expected: version1.Server{ + expectedServer: version1.Server{ SSL: true, SSLCertificate: "/etc/nginx/secrets/default-cafe-secret", SSLCertificateKey: "/etc/nginx/secrets/default-cafe-secret", }, - msg: "TLS termination", + expectedWarnings: Warnings{}, + msg: "TLS termination", }, { host: "cafe.example.com", @@ -911,12 +947,17 @@ func TestAddSSLConfig(t *testing.T) { }, }, isWildcardEnabled: false, - expected: version1.Server{ + expectedServer: version1.Server{ SSL: true, SSLCertificate: pemFileNameForMissingTLSSecret, SSLCertificateKey: pemFileNameForMissingTLSSecret, SSLCiphers: "NULL", }, + expectedWarnings: Warnings{ + nil: { + "TLS secret cafe-secret is invalid: invalid secret", + }, + }, msg: "invalid secret", }, { @@ -934,12 +975,17 @@ func TestAddSSLConfig(t *testing.T) { }, }, isWildcardEnabled: false, - expected: version1.Server{ + expectedServer: version1.Server{ SSL: true, SSLCertificate: pemFileNameForMissingTLSSecret, SSLCertificateKey: pemFileNameForMissingTLSSecret, SSLCiphers: "NULL", }, + expectedWarnings: Warnings{ + nil: { + "TLS secret cafe-secret is of a wrong type 'nginx.org/ca', must be 'kubernetes.io/tls'", + }, + }, msg: "secret of wrong type", }, { @@ -951,12 +997,13 @@ func TestAddSSLConfig(t *testing.T) { }, }, isWildcardEnabled: true, - expected: version1.Server{ + expectedServer: version1.Server{ SSL: true, SSLCertificate: pemFileNameForWildcardTLSSecret, SSLCertificateKey: pemFileNameForWildcardTLSSecret, }, - msg: "no secret name with wildcard enabled", + expectedWarnings: Warnings{}, + msg: "no secret name with wildcard enabled", }, { host: "cafe.example.com", @@ -967,24 +1014,33 @@ func TestAddSSLConfig(t *testing.T) { }, }, isWildcardEnabled: false, - expected: version1.Server{ + expectedServer: version1.Server{ SSL: true, SSLCertificate: pemFileNameForMissingTLSSecret, SSLCertificateKey: pemFileNameForMissingTLSSecret, SSLCiphers: "NULL", }, + expectedWarnings: Warnings{ + nil: { + "TLS termination for host 'cafe.example.com' requires specifying a TLS secret or configuring a global wildcard TLS secret", + }, + }, msg: "no secret name with wildcard disabled", }, } for _, test := range tests { - var result version1.Server + var server version1.Server - addSSLConfig(&result, test.host, test.tls, test.secretRefs, test.isWildcardEnabled) + // it is ok to use nil as the owner + warnings := addSSLConfig(&server, nil, test.host, test.tls, test.secretRefs, test.isWildcardEnabled) - if diff := cmp.Diff(test.expected, result); diff != "" { + if diff := cmp.Diff(test.expectedServer, server); diff != "" { t.Errorf("addSSLConfig() '%s' mismatch (-want +got):\n%s", test.msg, diff) } + if !reflect.DeepEqual(test.expectedWarnings, warnings) { + t.Errorf("addSSLConfig() returned %v but expected %v for the case of %s", warnings, test.expectedWarnings, test.msg) + } } } @@ -995,6 +1051,7 @@ func TestGenerateJWTConfig(t *testing.T) { redirectLocationName string expectedJWTAuth *version1.JWTAuth expectedRedirectLocation *version1.JWTRedirectLocation + expectedWarnings Warnings msg string }{ { @@ -1016,6 +1073,7 @@ func TestGenerateJWTConfig(t *testing.T) { Token: "$http_token", }, expectedRedirectLocation: nil, + expectedWarnings: Warnings{}, msg: "normal case", }, { @@ -1042,7 +1100,8 @@ func TestGenerateJWTConfig(t *testing.T) { Name: "@loc", LoginURL: "http://cafe.example.com/login", }, - msg: "normal case with login url", + expectedWarnings: Warnings{}, + msg: "normal case with login url", }, { secretRefs: map[string]*secrets.SecretReference{ @@ -1063,7 +1122,12 @@ func TestGenerateJWTConfig(t *testing.T) { Token: "$http_token", }, expectedRedirectLocation: nil, - msg: "invalid secret", + expectedWarnings: Warnings{ + nil: { + "JWK secret cafe-jwk is invalid: invalid secret", + }, + }, + msg: "invalid secret", }, { secretRefs: map[string]*secrets.SecretReference{ @@ -1084,12 +1148,17 @@ func TestGenerateJWTConfig(t *testing.T) { Token: "$http_token", }, expectedRedirectLocation: nil, - msg: "secret of wrong type", + expectedWarnings: Warnings{ + nil: { + "JWK secret cafe-jwk is of a wrong type 'nginx.org/ca', must be 'nginx.org/jwk'", + }, + }, + msg: "secret of wrong type", }, } for _, test := range tests { - jwtAuth, redirectLocation := generateJWTConfig(test.secretRefs, test.cfgParams, test.redirectLocationName) + jwtAuth, redirectLocation, warnings := generateJWTConfig(nil, test.secretRefs, test.cfgParams, test.redirectLocationName) if diff := cmp.Diff(test.expectedJWTAuth, jwtAuth); diff != "" { t.Errorf("generateJWTConfig() '%s' mismatch for jwtAuth (-want +got):\n%s", test.msg, diff) @@ -1097,5 +1166,8 @@ func TestGenerateJWTConfig(t *testing.T) { if diff := cmp.Diff(test.expectedRedirectLocation, redirectLocation); diff != "" { t.Errorf("generateJWTConfig() '%s' mismatch for redirectLocation (-want +got):\n%s", test.msg, diff) } + if !reflect.DeepEqual(test.expectedWarnings, warnings) { + t.Errorf("generateJWTConfig() returned %v but expected %v for the case of %s", warnings, test.expectedWarnings, test.msg) + } } } diff --git a/internal/configs/warnings.go b/internal/configs/warnings.go index 8e4e814070..082bc4370e 100644 --- a/internal/configs/warnings.go +++ b/internal/configs/warnings.go @@ -1,6 +1,10 @@ package configs -import "k8s.io/apimachinery/pkg/runtime" +import ( + "fmt" + + "k8s.io/apimachinery/pkg/runtime" +) // Warnings stores a list of warnings for a given runtime k8s object in a map type Warnings map[runtime.Object][]string @@ -15,3 +19,8 @@ func (w Warnings) Add(warnings Warnings) { w[k] = v } } + +// Adds a warning for the specified object. +func (w Warnings) AddWarningf(obj runtime.Object, msgFmt string, args ...interface{}) { + w[obj] = append(w[obj], fmt.Sprintf(msgFmt, args...)) +} diff --git a/internal/k8s/controller.go b/internal/k8s/controller.go index ad554f19fd..8ad39fbb6e 100644 --- a/internal/k8s/controller.go +++ b/internal/k8s/controller.go @@ -935,14 +935,14 @@ func (lbc *LoadBalancerController) processChanges(changes []ResourceChange) { if impl.IsMaster { mergeableIng := lbc.createMergeableIngresses(impl) - addOrUpdateErr := lbc.configurator.AddOrUpdateMergeableIngress(mergeableIng) - lbc.updateMergeableIngressStatusAndEvents(impl, addOrUpdateErr) + warnings, addOrUpdateErr := lbc.configurator.AddOrUpdateMergeableIngress(mergeableIng) + lbc.updateMergeableIngressStatusAndEvents(impl, warnings, addOrUpdateErr) } else { // for regular Ingress, validMinionPaths is nil ingEx := lbc.createIngressEx(impl.Ingress, impl.ValidHosts, nil) - addOrUpdateErr := lbc.configurator.AddOrUpdateIngress(ingEx) - lbc.updateRegularIngressStatusAndEvents(impl, addOrUpdateErr) + warnings, addOrUpdateErr := lbc.configurator.AddOrUpdateIngress(ingEx) + lbc.updateRegularIngressStatusAndEvents(impl, warnings, addOrUpdateErr) } } } else if c.Op == Delete { @@ -1068,15 +1068,15 @@ func (lbc *LoadBalancerController) updateResourcesStatusAndEvents(resources []Re lbc.updateVirtualServerStatusAndEvents(impl, warnings, operationErr) case *FullIngress: if impl.IsMaster { - lbc.updateMergeableIngressStatusAndEvents(impl, operationErr) + lbc.updateMergeableIngressStatusAndEvents(impl, warnings, operationErr) } else { - lbc.updateRegularIngressStatusAndEvents(impl, operationErr) + lbc.updateRegularIngressStatusAndEvents(impl, warnings, operationErr) } } } } -func (lbc *LoadBalancerController) updateMergeableIngressStatusAndEvents(fullIng *FullIngress, operationErr error) { +func (lbc *LoadBalancerController) updateMergeableIngressStatusAndEvents(fullIng *FullIngress, warnings configs.Warnings, operationErr error) { eventType := api_v1.EventTypeNormal eventTitle := "AddedOrUpdated" eventWarningMessage := "" @@ -1087,6 +1087,12 @@ func (lbc *LoadBalancerController) updateMergeableIngressStatusAndEvents(fullIng eventWarningMessage = fmt.Sprintf("with warning(s): %s", formatWarningMessages(fullIng.Warnings)) } + if messages, ok := warnings[fullIng.Ingress]; ok { + eventType = api_v1.EventTypeWarning + eventTitle = "AddedOrUpdatedWithWarning" + eventWarningMessage = fmt.Sprintf("%s; with warning(s): %v", eventWarningMessage, formatWarningMessages(messages)) + } + if operationErr != nil { eventType = api_v1.EventTypeWarning eventTitle = "AddedOrUpdatedWithError" @@ -1108,6 +1114,12 @@ func (lbc *LoadBalancerController) updateMergeableIngressStatusAndEvents(fullIng minionEventWarningMessage = fmt.Sprintf("with warning(s): %s", formatWarningMessages(minionChangeWarnings)) } + if messages, ok := warnings[fm.Ingress]; ok { + minionEventType = api_v1.EventTypeWarning + minionEventTitle = "AddedOrUpdatedWithWarning" + minionEventWarningMessage = fmt.Sprintf("%s; with warning(s): %v", minionEventWarningMessage, formatWarningMessages(messages)) + } + if operationErr != nil { minionEventType = api_v1.EventTypeWarning minionEventTitle = "AddedOrUpdatedWithError" @@ -1132,7 +1144,7 @@ func (lbc *LoadBalancerController) updateMergeableIngressStatusAndEvents(fullIng } } -func (lbc *LoadBalancerController) updateRegularIngressStatusAndEvents(fullIng *FullIngress, operationErr error) { +func (lbc *LoadBalancerController) updateRegularIngressStatusAndEvents(fullIng *FullIngress, warnings configs.Warnings, operationErr error) { eventType := api_v1.EventTypeNormal eventTitle := "AddedOrUpdated" eventWarningMessage := "" @@ -1143,6 +1155,12 @@ func (lbc *LoadBalancerController) updateRegularIngressStatusAndEvents(fullIng * eventWarningMessage = fmt.Sprintf("with warning(s): %s", formatWarningMessages(fullIng.Warnings)) } + if messages, ok := warnings[fullIng.Ingress]; ok { + eventType = api_v1.EventTypeWarning + eventTitle = "AddedOrUpdatedWithWarning" + eventWarningMessage = fmt.Sprintf("%s; with warning(s): %v", eventWarningMessage, formatWarningMessages(messages)) + } + if operationErr != nil { eventType = api_v1.EventTypeWarning eventTitle = "AddedOrUpdatedWithError" @@ -2690,8 +2708,8 @@ func (lbc *LoadBalancerController) handleAppProtectPolicyUpdate(pol *unstructure // VirtualServers don't support AppProtect policies ingressExes, mergeableIngresses, _ := lbc.createExtendedResources(resources) - updateErr := lbc.configurator.AddOrUpdateAppProtectResource(pol, ingressExes, mergeableIngresses) - lbc.updateResourcesStatusAndEvents(resources, configs.Warnings{}, updateErr) + warnings, updateErr := lbc.configurator.AddOrUpdateAppProtectResource(pol, ingressExes, mergeableIngresses) + lbc.updateResourcesStatusAndEvents(resources, warnings, updateErr) return updateErr } @@ -2700,8 +2718,8 @@ func (lbc *LoadBalancerController) handleAppProtectPolicyDeletion(key string, re // VirtualServers don't support AppProtect policies ingressExes, mergeableIngresses, _ := lbc.createExtendedResources(resources) - deleteErr := lbc.configurator.DeleteAppProtectPolicy(key, ingressExes, mergeableIngresses) - lbc.updateResourcesStatusAndEvents(resources, configs.Warnings{}, deleteErr) + warnnigs, deleteErr := lbc.configurator.DeleteAppProtectPolicy(key, ingressExes, mergeableIngresses) + lbc.updateResourcesStatusAndEvents(resources, warnnigs, deleteErr) return deleteErr } @@ -2760,8 +2778,8 @@ func (lbc *LoadBalancerController) appProtectLogConfUpdate(logConf *unstructured // VirtualServers don't support AppProtectLogConf ingressExes, mergeableIngresses, _ := lbc.createExtendedResources(resources) - updateErr := lbc.configurator.AddOrUpdateAppProtectResource(logConf, ingressExes, mergeableIngresses) - lbc.updateResourcesStatusAndEvents(resources, configs.Warnings{}, updateErr) + warnings, updateErr := lbc.configurator.AddOrUpdateAppProtectResource(logConf, ingressExes, mergeableIngresses) + lbc.updateResourcesStatusAndEvents(resources, warnings, updateErr) return updateErr } @@ -2770,8 +2788,8 @@ func (lbc *LoadBalancerController) appProtectLogConfDeletion(key string, resourc // VirtualServers don't support AppProtectLogConf ingressExes, mergeableIngresses, _ := lbc.createExtendedResources(resources) - deleteErr := lbc.configurator.DeleteAppProtectLogConf(key, ingressExes, mergeableIngresses) - lbc.updateResourcesStatusAndEvents(resources, configs.Warnings{}, deleteErr) + warnings, deleteErr := lbc.configurator.DeleteAppProtectLogConf(key, ingressExes, mergeableIngresses) + lbc.updateResourcesStatusAndEvents(resources, warnings, deleteErr) return deleteErr }