8000 AP: add support for multiple log configs · nginx/kubernetes-ingress@da75b10 · GitHub
[go: up one dir, main page]

Skip to content

Commit da75b10

Browse files
author
Rafal Wegrzycki
committed
AP: add support for multiple log configs
1 parent ea2e9f9 commit da75b10

File tree

8 files changed

+72
-41
lines changed

8 files changed

+72
-41
lines changed

internal/configs/configurator.go

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1221,20 +1221,21 @@ func createSpiffeCert(certs []*x509.Certificate) []byte {
12211221
return pemData
12221222
}
12231223

1224-
func (cnf *Configurator) updateApResources(ingEx *IngressEx) map[string]string {
1225-
apRes := make(map[string]string)
1224+
func (cnf *Configurator) updateApResources(ingEx *IngressEx) (apRes AppProtectResources) {
12261225
if ingEx.AppProtectPolicy != nil {
12271226
policyFileName := appProtectPolicyFileNameFromUnstruct(ingEx.AppProtectPolicy)
12281227
policyContent := generateApResourceFileContent(ingEx.AppProtectPolicy)
12291228
cnf.nginxManager.CreateAppProtectResourceFile(policyFileName, policyContent)
1230-
apRes[appProtectPolicyKey] = policyFileName
1229+
apRes.AppProtectPolicy = policyFileName
12311230
}
12321231

12331232
if ingEx.AppProtectLogConf != nil {
1234-
logConfFileName := appProtectLogConfFileNameFromUnstruct(ingEx.AppProtectLogConf)
1235-
logConfContent := generateApResourceFileContent(ingEx.AppProtectLogConf)
1236-
cnf.nginxManager.CreateAppProtectResourceFile(logConfFileName, logConfContent)
1237-
apRes[appProtectLogConfKey] = logConfFileName + " " + ingEx.AppProtectLogDst
1233+
for i, logConf := range ingEx.AppProtectLogConf {
1234+
logConfFileName := appProtectLogConfFileNameFromUnstruct(logConf)
1235+
logConfContent := generateApResourceFileContent(logConf)
1236+
cnf.nginxManager.CreateAppProtectResourceFile(logConfFileName, logConfContent)
1237+
apRes.AppProtectLogconfs = append(apRes.AppProtectLogconfs, logConfFileName+" "+ingEx.AppProtectLogDst[i])
1238+
}
12381239
}
12391240

12401241
return apRes

internal/configs/ingress.go

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,12 @@ const emptyHost = ""
2020
const appProtectPolicyKey = "policy"
2121
const appProtectLogConfKey = "logconf"
2222

23+
// AppProtectResources holds namespace names of App Protect resources relavant to an Ingress
24+
type AppProtectResources struct {
25+
AppProtectPolicy string
26+
AppProtectLogconfs []string
27+
}
28+
2329
// IngressEx holds an Ingress along with the resources that are referenced in this Ingress.
2430
type IngressEx struct {
2531
Ingress *networking.Ingress
@@ -30,8 +36,8 @@ type IngressEx struct {
3036
ValidHosts map[string]bool
3137
ValidMinionPaths map[string]bool
3238
AppProtectPolicy *unstructured.Unstructured
33-
AppProtectLogConf *unstructured.Unstructured
34-
AppProtectLogDst string
39+
AppProtectLogConf []*unstructured.Unstructured
40+
AppProtectLogDst []string
3541
SecretRefs map[string]*secrets.SecretReference
3642
}
3743

@@ -55,7 +61,7 @@ type MergeableIngresses struct {
5561
Minions []*IngressEx
5662
}
5763

58-
func generateNginxCfg(ingEx *IngressEx, apResources map[string]string, isMinion bool, baseCfgParams *ConfigParams, isPlus bool,
64+
func generateNginxCfg(ingEx *IngressEx, apResources AppProtectResources, isMinion bool, baseCfgParams *ConfigParams, isPlus bool,
5965
isResolverConfigured bool, staticParams *StaticConfigParams, isWildcardEnabled bool) (version1.IngressNginxConfig, Warnings) {
6066
hasAppProtect := staticParams.MainAppProtectLoadModule
6167
cfgParams := parseAnnotations(ingEx, baseCfgParams, isPlus, hasAppProtect, staticParams.EnableInternalRoutes)
@@ -139,8 +145,8 @@ func generateNginxCfg(ingEx *IngressEx, apResources map[string]string, isMinion
139145
allWarnings.Add(warnings)
140146

141147
if hasAppProtect {
142-
server.AppProtectPolicy = apResources[appProtectPolicyKey]
143-
server.AppProtectLogConf = apResources[appProtectLogConfKey]
148+
server.AppProtectPolicy = apResources.AppProtectPolicy
149+
server.AppProtectLogConfs = apResources.AppProtectLogconfs
144150
}
145151

146152
if !isMinion && cfgParams.JWTKey != "" {
@@ -501,7 +507,7 @@ func upstreamMapToSlice(upstreams map[string]version1.Upstream) []version1.Upstr
501507
return result
502508
}
503509

504-
func generateNginxCfgForMergeableIngresses(mergeableIngs *MergeableIngresses, masterApResources map[string]string,
510+
func generateNginxCfgForMergeableIngresses(mergeableIngs *MergeableIngresses, masterApResources AppProtectResources,
505511
baseCfgParams *ConfigParams, isPlus bool, isResolverConfigured bool, staticParams *StaticConfigParams,
506512
isWildcardEnabled bool) (version1.IngressNginxConfig, Warnings) {
507513

@@ -559,8 +565,8 @@ func generateNginxCfgForMergeableIngresses(mergeableIngs *MergeableIngresses, ma
559565
}
560566

561567
isMinion := true
562-
// App Protect Resources not allowed in minions - pass empty map
563-
dummyApResources := make(map[string]string)
568+
// App Protect Resources not allowed in minions - pass empty struct
569+
dummyApResources := AppProtectResources{}
564570
nginxCfg, minionWarnings := generateNginxCfg(minion, dummyApResources, isMinion, baseCfgParams, isPlus, isResolverConfigured, staticParams, isWildcardEnabled)
565571
warnings.Add(minionWarnings)
566572

internal/configs/ingress_test.go

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ func TestGenerateNginxCfg(t *testing.T) {
2222

2323
expected := createExpectedConfigForCafeIngressEx()
2424

25-
apRes := make(map[string]string)
25+
apRes := AppProtectResources{}
2626
result, warnings := generateNginxCfg(&cafeIngressEx, apRes, false, configParams, false, false, &StaticConfigParams{}, false)
2727

2828
if diff := cmp.Diff(expected, result); diff != "" {
@@ -62,7 +62,7 @@ func TestGenerateNginxCfgForJWT(t *testing.T) {
6262
},
6363
}
6464

65-
apRes := make(map[string]string)
65+
apRes := AppProtectResources{}
6666
result, warnings := generateNginxCfg(&cafeIngressEx, apRes, false, configParams, true, false, &StaticConfigParams{}, false)
6767

6868
if !reflect.DeepEqual(result.Servers[0].JWTAuth, expected.Servers[0].JWTAuth) {
@@ -81,7 +81,7 @@ func TestGenerateNginxCfgWithMissingTLSSecret(t *testing.T) {
8181
cafeIngressEx.SecretRefs["cafe-secret"].Error = errors.New("secret doesn't exist")
8282
configParams := NewDefaultConfigParams()
8383

84-
apRes := make(map[string]string)
84+
apRes := AppProtectResources{}
8585
result, resultWarnings := generateNginxCfg(&cafeIngressEx, apRes, false, configParams, false, false, &StaticConfigParams{}, false)
8686

8787
expectedCiphers := "NULL"
@@ -105,7 +105,7 @@ func TestGenerateNginxCfgWithWildcardTLSSecret(t *testing.T) {
105105
cafeIngressEx.Ingress.Spec.TLS[0].SecretName = ""
106106
configParams := NewDefaultConfigParams()
107107

108-
apRes := make(map[string]string)
108+
apRes := AppProtectResources{}
109109
result, warnings := generateNginxCfg(&cafeIngressEx, apRes, false, configParams, false, false, &StaticConfigParams{}, true)
110110

111111
resultServer := result.Servers[0]
@@ -328,7 +328,7 @@ func TestGenerateNginxCfgForMergeableIngresses(t *testing.T) {
328328

329329
configParams := NewDefaultConfigParams()
330330

331-
masterApRes := make(map[string]string)
331+
masterApRes := AppProtectResources{}
332332
result, warnings := generateNginxCfgForMergeableIngresses(mergeableIngresses, masterApRes, configParams, false, false, &StaticConfigParams{}, false)
333333

334334
if diff := cmp.Diff(expected, result); diff != "" {
@@ -353,7 +353,7 @@ func TestGenerateNginxConfigForCrossNamespaceMergeableIngresses(t *testing.T) {
353353
expected := createExpectedConfigForCrossNamespaceMergeableCafeIngress()
354354
configParams := NewDefaultConfigParams()
355355

356-
emptyApResources := make(map[string]string)
356+
emptyApResources := AppProtectResources{}
357357
result, warnings := generateNginxCfgForMergeableIngresses(mergeableIngresses, emptyApResources, configParams, false, false, &StaticConfigParams{}, false)
358358

359359
if diff := cmp.Diff(expected, result); diff != "" {
@@ -417,7 +417,7 @@ func TestGenerateNginxCfgForMergeableIngressesForJWT(t *testing.T) {
417417
configParams := NewDefaultConfigParams()
418418
isPlus := true
419419

420-
masterApRes := make(map[string]string)
420+
masterApRes := AppProtectResources{}
421421
result, warnings := generateNginxCfgForMergeableIngresses(mergeableIngresses, masterApRes, configParams, isPlus, false, &StaticConfigParams{}, false)
422422 67DE

423423
if !reflect.DeepEqual(result.Servers[0].JWTAuth, expected.Servers[0].JWTAuth) {
@@ -792,7 +792,7 @@ func TestGenerateNginxCfgForSpiffe(t *testing.T) {
792792
expected.Servers[0].Locations[i].SSL = true
793793
}
794794

795-
apResources := make(map[string]string)
795+
apResources := AppProtectResources{}
796796
result, warnings := generateNginxCfg(&cafeIngressEx, apResources, false, configParams, false, false,
797797
&StaticConfigParams{NginxServiceMesh: true}, false)
798798

@@ -814,7 +814,7 @@ func TestGenerateNginxCfgForInternalRoute(t *testing.T) {
814814
expected.Servers[0].SpiffeCerts = true
815815
expected.Ingress.Annotations[internalRouteAnnotation] = "true"
816816

817-
apResources := make(map[string]string)
817+
apResources := AppProtectResources{}
818818
result, warnings := generateNginxCfg(&cafeIngressEx, apResources, false, configParams, false, false,
819819
&StaticConfigParams{NginxServiceMesh: true, EnableInternalRoutes: true}, false)
820820

internal/configs/version1/config.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ type Server struct {
9797
SSLPorts []int
9898
AppProtectEnable string
9999
AppProtectPolicy string
100-
AppProtectLogConf string
100+
AppProtectLogConfs []string
101101
AppProtectLogEnable string
102102

103103
SpiffeCerts bool

internal/configs/version1/nginx-plus.ingress.tmpl

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,8 @@ server {
6969
{{- end}}
7070
{{- if $server.AppProtectLogEnable}}
7171
app_protect_security_log_enable {{$server.AppProtectLogEnable}};
72-
{{if $server.AppProtectLogConf}}app_protect_security_log {{$server.AppProtectLogConf}};{{end}}
72+
{{if $server.AppProtectLogConfs}}{{range $AppProtectLogConf := $server.AppProtectLogConfs}}app_protect_security_log {{$AppProtectLogConf}};
73+
{{end}}{{end}}
7374
{{- end}}
7475

7576
{{if not $server.GRPCOnly}}

internal/k8s/appprotect/app_protect_resources.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,15 @@ func ParseResourceReferenceAnnotation(ns, antn string) string {
131131
return antn
132132
}
133133

134+
// ParseResourceReferenceAnnotationList returns a slice of ns/names strings
135+
func ParseResourceReferenceAnnotationList(ns, antns string) []string {
136+
out := []string{}
137+
for _, antn := range strings.Split(antns, ",") {
138+
out = append(out, ParseResourceReferenceAnnotation(ns, antn))
139+
}
140+
return out
141+
}
142+
134143
func validateAppProtectUserSig(userSig *unstructured.Unstructured) error {
135144
sigName := userSig.GetName()
136145
err := validateRequiredSlices(userSig, appProtectUserSigRequiredSlices)

internal/k8s/controller.go

Lines changed: 20 additions & 12 deletions
CDAC
Original file line numberDiff line numberDiff line change
@@ -2050,7 +2050,7 @@ func (lbc *LoadBalancerController) createIngressEx(ing *networking.Ingress, vali
20502050
if apLogConfAntn, exists := ingEx.Ingress.Annotations[configs.AppProtectLogConfAnnotation]; exists {
20512051
logConf, logDst, err := lbc.getAppProtectLogConfAndDst(ing)
20522052
if err != nil {
2053-
glog.Warningf("Error Getting App Protect policy %v for Ingress %v/%v: %v", apLogConfAntn, ing.Namespace, ing.Name, err)
2053+
glog.Warningf("Error Getting App Protect Log Comnfig %v for Ingress %v/%v: %v", apLogConfAntn, ing.Namespace, ing.Name, err)
20542054
} else {
20552055
ingEx.AppProtectLogConf = logConf
20562056
ingEx.AppProtectLogDst = logDst
@@ -2163,27 +2163,35 @@ func (lbc *LoadBalancerController) createIngressEx(ing *networking.Ingress, vali
21632163
return ingEx
21642164
}
21652165

2166-
func (lbc *LoadBalancerController) getAppProtectLogConfAndDst(ing *networking.Ingress) (logConf *unstructured.Unstructured, logDst string, err error) {
2167-
logConfNsN := appprotect.ParseResourceReferenceAnnotation(ing.Namespace, ing.Annotations[configs.AppProtectLogConfAnnotation])
2166+
func (lbc *LoadBalancerController) getAppProtectLogConfAndDst(ing *networking.Ingress) (logConfs []*unstructured.Unstructured, logDsts []string, err error) {
21682167

21692168
if _, exists := ing.Annotations[configs.AppProtectLogConfDstAnnotation]; !exists {
2170-
return nil, "", fmt.Errorf("Error: %v requires %v in %v", configs.AppProtectLogConfAnnotation, configs.AppProtectLogConfDstAnnotation, ing.Name)
2169+
return nil, []string{""}, fmt.Errorf("Error: %v requires %v in %v", configs.AppProtectLogConfAnnotation, configs.AppProtectLogConfDstAnnotation, ing.Name)
21712170
}
21722171

2173-
logDst = ing.Annotations[configs.AppProtectLogConfDstAnnotation]
2172+
logDsts = strings.Split(ing.Annotations[configs.AppProtectLogConfDstAnnotation], ",")
2173+
logConfNsNs := appprotect.ParseResourceReferenceAnnotationList(ing.Namespace, ing.Annotations[configs.AppProtectLogConfAnnotation])
2174+
if len(logDsts) != len(logConfNsNs) {
2175+
return nil, []string{""}, fmt.Errorf("Error Validating App Protect Destination and config for Ingress %v: LogConf and LogDestination must be equal length", ing.Name)
2176+
}
21742177

2175-
err = appprotect.ValidateAppProtectLogDestination(logDst)
2178+
for _, logDst := range logDsts {
2179+
err = appprotect.ValidateAppProtectLogDestination(logDst)
21762180

2177-
if err != nil {
2178-
return nil, "", fmt.Errorf("Error Validating App Protect Destination Config for Ingress %v: %v", ing.Name, err)
2181+
if err != nil {
2182+
return nil, []string{""}, fmt.Errorf("Error Validating App Protect Destination Config for Ingress %v: %v", ing.Name, err)
2183+
}
21792184
}
21802185

2181-
logConf, err = lbc.appProtectConfiguration.GetAppResource(appprotect.LogConfGVK.Kind, logConfNsN)
2182-
if err != nil {
2183-
return nil, "", fmt.Errorf("Error retrieving App Protect Log Config for Ingress %v: %v", ing.Name, err)
2186+
for _, logConfNsN := range logConfNsNs {
2187+
logConf, err := lbc.appProtectConfiguration.GetAppResource(appprotect.LogConfGVK.Kind, logConfNsN)
2188+
if err != nil {
2189+
return nil, []string{""}, fmt.Errorf("Error retrieving App Protect Log Config for Ingress %v: %v", ing.Name, err)
2190+
}
2191+
logConfs = append(logConfs, logConf)
21842192
}
21852193

2186-
return logConf, logDst, nil
2194+
return logConfs, logDsts, nil
21872195
}
21882196

21892197
func (lbc *LoadBalancerController) getAppProtectPolicy(ing *networking.Ingress) (apPolicy *unstructured.Unstructured, err error) {

internal/k8s/reference_checkers.go

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ import (
55
v1 "github.com/nginxinc/kubernetes-ingress/pkg/apis/configuration/v1"
66
conf_v1alpha1 "github.com/nginxinc/kubernetes-ingress/pkg/apis/configuration/v1alpha1"
77
networking "k8s.io/api/networking/v1beta1"
8+
9+
"strings"
810
)
911

1012
type resourceReferenceChecker interface {
@@ -217,10 +219,14 @@ func newAppProtectResourceReferenceChecker(annotation string) *appProtectResourc
217219
return &appProtectResourceReferenceChecker{annotation}
218220
}
219221

222+
// In App Protect logConfs can be a coma separated list.
220223
func (rc *appProtectResourceReferenceChecker) IsReferencedByIngress(namespace string, name string, ing *networking.Ingress) bool {
221-
if pol, exists := ing.Annotations[rc.annotation]; exists {
222-
if pol == namespace+"/"+name || (namespace == ing.Namespace && pol == name) {
223-
return true
224+
if resName, exists := ing.Annotations[rc.annotation]; exists {
225+
resNames := strings.Split(resName, ",")
226+
for _, res := range resNames {
227+
if res == namespace+"/"+name || (namespace == ing.Namespace && res == name) {
228+
return true
229+
}
224230
}
225231
}
226232
return false

0 commit comments

Comments
 (0)
0