Skip to content

Commit

Permalink
AP: Deprecate exteral refs
Browse files Browse the repository at this point in the history
  • Loading branch information
Rafal Wegrzycki committed Feb 28, 2022
1 parent e15f56d commit f49b9a1
Show file tree
Hide file tree
Showing 14 changed files with 150 additions and 48 deletions.
4 changes: 3 additions & 1 deletion docs/content/app-protect/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,9 @@ You can define App Protect policies for your Ingress resources by creating an `A
> **Note**: [The Advanced gRPC Protection for Unary Traffic](/nginx-app-protect/configuration/#advanced-grpc-protection-for-unary-traffic) only supports providing an `idl-file` inline. The fields `policy.idl-files[].link`, `policy.idl-files[].$ref`, and
`policy.idl-files[].file` are not supported. The IDL file should be provided in field `policy.idl-files[].contents`. The value of this field can be base64 encoded. In this case the field `policy.idl-files[].isBase64` should be set to `true`.

To add any [App Protect policy](/nginx-app-protect/declarative-policy/policy/) to an Ingress resource:
> **Note**: [External References](/nginx-app-protect/configuration-guide/configuration/#external-references) in the Ingress Controller are deprecated and will not be supported in future releases.
To add any [App Protect policy](/nginx-app-protect/policy/#policy) to an Ingress resource:

1. Create an `APPolicy` Custom resource manifest.
2. Add the desired policy to the `spec` field in the `APPolicy` resource.
Expand Down
4 changes: 2 additions & 2 deletions internal/configs/configurator.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,8 @@ type Configurator struct {
// NewConfigurator creates a new Configurator.
func NewConfigurator(nginxManager nginx.Manager, staticCfgParams *StaticConfigParams, config *ConfigParams,
templateExecutor *version1.TemplateExecutor, templateExecutorV2 *version2.TemplateExecutor, isPlus bool, isWildcardEnabled bool,
labelUpdater collector.LabelUpdater, isPrometheusEnabled bool, latencyCollector latCollector.LatencyCollector, isLatencyMetricsEnabled bool) *Configurator {
labelUpdater collector.LabelUpdater, isPrometheusEnabled bool, latencyCollector latCollector.LatencyCollector, isLatencyMetricsEnabled bool,
) *Configurator {
metricLabelsIndex := &metricLabelsIndex{
ingressUpstreams: make(map[string][]string),
virtualServerUpstreams: make(map[string][]string),
Expand Down Expand Up @@ -1445,7 +1446,6 @@ func (cnf *Configurator) DeleteAppProtectLogConf(resource *unstructured.Unstruct
func (cnf *Configurator) RefreshAppProtectUserSigs(
userSigs []*unstructured.Unstructured, delPols []string, ingExes []*IngressEx, mergeableIngresses []*MergeableIngresses, vsExes []*VirtualServerEx,
) (Warnings, error) {

allWarnings, err := cnf.addOrUpdateIngressesAndVirtualServers(ingExes, mergeableIngresses, vsExes)
if err != nil {
return allWarnings, err
Expand Down
16 changes: 10 additions & 6 deletions internal/configs/ingress.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,8 @@ type MergeableIngresses struct {
}

func generateNginxCfg(ingEx *IngressEx, apResources *AppProtectResources, dosResource *appProtectDosResource, isMinion bool,
baseCfgParams *ConfigParams, isPlus bool, isResolverConfigured bool, staticParams *StaticConfigParams, isWildcardEnabled bool) (version1.IngressNginxConfig, Warnings) {
baseCfgParams *ConfigParams, isPlus bool, isResolverConfigured bool, staticParams *StaticConfigParams, isWildcardEnabled bool,
) (version1.IngressNginxConfig, Warnings) {
hasAppProtect := staticParams.MainAppProtectLoadModule
hasAppProtectDos := staticParams.MainAppProtectDosLoadModule

Expand Down Expand Up @@ -290,7 +291,8 @@ func generateNginxCfg(ingEx *IngressEx, apResources *AppProtectResources, dosRes
}

func generateJWTConfig(owner runtime.Object, secretRefs map[string]*secrets.SecretReference, cfgParams *ConfigParams,
redirectLocationName string) (*version1.JWTAuth, *version1.JWTRedirectLocation, Warnings) {
redirectLocationName string,
) (*version1.JWTAuth, *version1.JWTRedirectLocation, Warnings) {
warnings := newWarnings()

secretRef := secretRefs[cfgParams.JWTKey]
Expand Down Expand Up @@ -326,7 +328,8 @@ func generateJWTConfig(owner runtime.Object, secretRefs map[string]*secrets.Secr
}

func addSSLConfig(server *version1.Server, owner runtime.Object, host string, ingressTLS []networking.IngressTLS,
secretRefs map[string]*secrets.SecretReference, isWildcardEnabled bool) Warnings {
secretRefs map[string]*secrets.SecretReference, isWildcardEnabled bool,
) Warnings {
warnings := newWarnings()

var tlsEnabled bool
Expand Down Expand Up @@ -427,7 +430,8 @@ func upstreamRequiresQueue(name string, ingEx *IngressEx, cfg *ConfigParams) (n
}

func createUpstream(ingEx *IngressEx, name string, backend *networking.IngressBackend, stickyCookie string, cfg *ConfigParams,
isPlus bool, isResolverConfigured bool, isLatencyMetricsEnabled bool) version1.Upstream {
isPlus bool, isResolverConfigured bool, isLatencyMetricsEnabled bool,
) version1.Upstream {
var ups version1.Upstream
labels := version1.UpstreamLabels{
Service: backend.Service.Name,
Expand Down Expand Up @@ -536,8 +540,8 @@ func upstreamMapToSlice(upstreams map[string]version1.Upstream) []version1.Upstr

func generateNginxCfgForMergeableIngresses(mergeableIngs *MergeableIngresses, apResources *AppProtectResources,
dosResource *appProtectDosResource, baseCfgParams *ConfigParams, isPlus bool, isResolverConfigured bool,
staticParams *StaticConfigParams, isWildcardEnabled bool) (version1.IngressNginxConfig, Warnings) {

staticParams *StaticConfigParams, isWildcardEnabled bool,
) (version1.IngressNginxConfig, Warnings) {
var masterServer version1.Server
var locations []version1.Location
var upstreams []version1.Upstream
Expand Down
1 change: 0 additions & 1 deletion internal/configs/version1/templates_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ var (
)

var ingCfg = IngressNginxConfig{

Servers: []Server{
{
Name: "test.example.com",
Expand Down
15 changes: 10 additions & 5 deletions internal/configs/virtualserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -1575,7 +1575,8 @@ type errorPageDetails struct {
func generateLocation(path string, upstreamName string, upstream conf_v1.Upstream, action *conf_v1.Action,
cfgParams *ConfigParams, errorPages errorPageDetails, internal bool, proxySSLName string,
originalPath string, locSnippets string, enableSnippets bool, retLocIndex int, isVSR bool, vsrName string,
vsrNamespace string, vscWarnings Warnings) (version2.Location, *version2.ReturnLocation) {
vsrNamespace string, vscWarnings Warnings,
) (version2.Location, *version2.ReturnLocation) {
locationSnippets := generateSnippets(enableSnippets, locSnippets, cfgParams.LocationSnippets)

if action.Redirect != nil {
Expand Down Expand Up @@ -1674,7 +1675,8 @@ func generateProxyAddHeaders(proxy *conf_v1.ActionProxy) []version2.AddHeader {

func generateLocationForProxying(path string, upstreamName string, upstream conf_v1.Upstream,
cfgParams *ConfigParams, errorPages []conf_v1.ErrorPage, internal bool, errPageIndex int,
proxySSLName string, proxy *conf_v1.ActionProxy, originalPath string, locationSnippets []string, isVSR bool, vsrName string, vsrNamespace string) version2.Location {
proxySSLName string, proxy *conf_v1.ActionProxy, originalPath string, locationSnippets []string, isVSR bool, vsrName string, vsrNamespace string,
) version2.Location {
return version2.Location{
Path: generatePath(path),
Internal: internal,
Expand Down Expand Up @@ -1741,7 +1743,8 @@ func generateLocationForRedirect(
}

func generateLocationForReturn(path string, locationSnippets []string, actionReturn *conf_v1.ActionReturn,
retLocIndex int) (version2.Location, *version2.ReturnLocation) {
retLocIndex int,
) (version2.Location, *version2.ReturnLocation) {
defaultType := actionReturn.Type
if defaultType == "" {
defaultType = "text/plain"
Expand Down Expand Up @@ -1873,7 +1876,8 @@ func generateDefaultSplitsConfig(

func generateMatchesConfig(route conf_v1.Route, upstreamNamer *upstreamNamer, crUpstreams map[string]conf_v1.Upstream,
variableNamer *variableNamer, index int, scIndex int, cfgParams *ConfigParams, errorPages errorPageDetails,
locSnippets string, enableSnippets bool, retLocIndex int, isVSR bool, vsrName string, vsrNamespace string, vscWarnings Warnings) routingCfg {
locSnippets string, enableSnippets bool, retLocIndex int, isVSR bool, vsrName string, vsrNamespace string, vscWarnings Warnings,
) routingCfg {
// Generate maps
var maps []version2.Map

Expand Down Expand Up @@ -2101,7 +2105,8 @@ func getNameForSourceForMatchesRouteMapFromCondition(condition conf_v1.Condition
}

func (vsc *virtualServerConfigurator) generateSSLConfig(owner runtime.Object, tls *conf_v1.TLS, namespace string,
secretRefs map[string]*secrets.SecretReference, cfgParams *ConfigParams) *version2.SSL {
secretRefs map[string]*secrets.SecretReference, cfgParams *ConfigParams,
) *version2.SSL {
if tls == nil {
return nil
}
Expand Down
8 changes: 0 additions & 8 deletions internal/configs/virtualserver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6673,7 +6673,6 @@ func TestGenerateHealthCheck(t *testing.T) {
msg string
}{
{

upstream: conf_v1.Upstream{
HealthCheck: &conf_v1.HealthCheck{
Enable: true,
Expand Down Expand Up @@ -6854,7 +6853,6 @@ func TestGenerateGrpcHealthCheck(t *testing.T) {
msg string
}{
{

upstream: conf_v1.Upstream{
HealthCheck: &conf_v1.HealthCheck{
Enable: true,
Expand Down Expand Up @@ -8174,7 +8172,6 @@ func TestAddWafConfig(t *testing.T) {
msg string
}{
{

wafInput: &conf_v1.WAF{
Enable: true,
},
Expand All @@ -8191,7 +8188,6 @@ func TestAddWafConfig(t *testing.T) {
msg: "valid waf config, default App Protect config",
},
{

wafInput: &conf_v1.WAF{
Enable: true,
ApPolicy: "dataguard-alarm",
Expand Down Expand Up @@ -8220,7 +8216,6 @@ func TestAddWafConfig(t *testing.T) {
msg: "valid waf config",
},
{

wafInput: &conf_v1.WAF{
Enable: true,
ApPolicy: "default/dataguard-alarm",
Expand Down Expand Up @@ -8252,7 +8247,6 @@ func TestAddWafConfig(t *testing.T) {
msg: "invalid waf config, apLogConf references non-existing log conf",
},
{

wafInput: &conf_v1.WAF{
Enable: true,
ApPolicy: "default/dataguard-alarm",
Expand Down Expand Up @@ -8283,7 +8277,6 @@ func TestAddWafConfig(t *testing.T) {
msg: "invalid waf config, apLogConf references non-existing ap conf",
},
{

wafInput: &conf_v1.WAF{
Enable: true,
ApPolicy: "ns1/dataguard-alarm",
Expand Down Expand Up @@ -8312,7 +8305,6 @@ func TestAddWafConfig(t *testing.T) {
msg: "valid waf config, cross ns reference",
},
{

wafInput: &conf_v1.WAF{
Enable: false,
ApPolicy: "dataguard-alarm",
Expand Down
4 changes: 2 additions & 2 deletions internal/k8s/appprotect/app_protect_configuration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -567,7 +567,7 @@ func TestAddOrUpdatePolicy(t *testing.T) {
{
Object: invalidTestPolicy,
Reason: "Rejected",
Message: "Error validating policy : Error validating App Protect Policy : Required field map[] not found",
Message: "Error validating policy : error validating App Protect Policy : required field map[] not found",
},
},
msg: "validation failed",
Expand Down Expand Up @@ -674,7 +674,7 @@ func TestAddOrUpdateLogConf(t *testing.T) {
{
Object: invalidLogConf,
Reason: "Rejected",
Message: "Error validating App Protect Log Configuration testlogconf: Required field map[] not found",
Message: "error validating App Protect Log Configuration testlogconf: required field map[] not found",
},
},
msg: "validation failed",
Expand Down
12 changes: 6 additions & 6 deletions internal/k8s/appprotectdos/app_protect_dos_configuration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ func TestCreateAppProtectDosPolicyEx(t *testing.T) {
},
expectedPolicyEx: &DosPolicyEx{
IsValid: false,
ErrorMsg: "failed to store ApDosPolicy: error validating DosPolicy : Required field map[] not found",
ErrorMsg: "failed to store ApDosPolicy: error validating DosPolicy : required field map[] not found",
},
wantErr: true,
msg: "dos policy no spec",
Expand Down Expand Up @@ -90,7 +90,7 @@ func TestCreateAppProtectDosLogConfEx(t *testing.T) {
},
expectedLogConfEx: &DosLogConfEx{
IsValid: false,
ErrorMsg: "failed to store ApDosLogconf: error validating App Protect Dos Log Configuration : Required field map[] not found",
ErrorMsg: "failed to store ApDosLogconf: error validating App Protect Dos Log Configuration : required field map[] not found",
},
wantErr: true,
msg: "Invalid DosLogConf",
Expand Down Expand Up @@ -259,7 +259,7 @@ func TestAddOrUpdateDosPolicy(t *testing.T) {
Resource: &DosPolicyEx{
Obj: invalidTestPolicy,
IsValid: false,
ErrorMsg: "failed to store ApDosPolicy: error validating DosPolicy : Required field map[] not found",
ErrorMsg: "failed to store ApDosPolicy: error validating DosPolicy : required field map[] not found",
},
Op: Delete,
},
Expand All @@ -268,7 +268,7 @@ func TestAddOrUpdateDosPolicy(t *testing.T) {
{
Object: invalidTestPolicy,
Reason: "Rejected",
Message: "error validating DosPolicy : Required field map[] not found",
Message: "error validating DosPolicy : required field map[] not found",
},
},
msg: "validation failed",
Expand Down Expand Up @@ -358,7 +358,7 @@ func TestAddOrUpdateDosLogConf(t *testing.T) {
Resource: &DosLogConfEx{
Obj: invalidLogConf,
IsValid: false,
ErrorMsg: "failed to store ApDosLogconf: error validating App Protect Dos Log Configuration invalid-logconf: Required field map[] not found",
ErrorMsg: "failed to store ApDosLogconf: error validating App Protect Dos Log Configuration invalid-logconf: required field map[] not found",
},
Op: Delete,
},
Expand All @@ -367,7 +367,7 @@ func TestAddOrUpdateDosLogConf(t *testing.T) {
{
Object: invalidLogConf,
Reason: "Rejected",
Message: "error validating App Protect Dos Log Configuration invalid-logconf: Required field map[] not found",
Message: "error validating App Protect Dos Log Configuration invalid-logconf: required field map[] not found",
},
},
msg: "validation failed",
Expand Down
6 changes: 4 additions & 2 deletions internal/k8s/configuration.go
Original file line number Diff line number Diff line change
Expand Up @@ -1190,7 +1190,8 @@ func createResourceChangesForHosts(removedHosts []string, updatedHosts []string,
}

func createResourceChangesForListeners(removedListeners []string, updatedListeners []string, addedListeners []string, oldListeners map[string]*TransportServerConfiguration,
newListeners map[string]*TransportServerConfiguration) []ResourceChange {
newListeners map[string]*TransportServerConfiguration,
) []ResourceChange {
var changes []ResourceChange
var deleteChanges []ResourceChange

Expand Down Expand Up @@ -1597,7 +1598,8 @@ func detectChangesInHosts(oldHosts map[string]Resource, newHosts map[string]Reso
}

func detectChangesInListeners(oldListeners map[string]*TransportServerConfiguration, newListeners map[string]*TransportServerConfiguration) (removedListeners []string,
updatedListeners []string, addedListeners []string) {
updatedListeners []string, addedListeners []string,
) {
for _, l := range getSortedTransportServerConfigurationKeys(oldListeners) {
_, exists := newListeners[l]
if !exists {
Expand Down
62 changes: 59 additions & 3 deletions pkg/apis/configuration/validation/appprotect.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@ package validation

import (
"fmt"
"strings"

"github.com/golang/glog"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
)

Expand All @@ -19,13 +21,52 @@ var appProtectUserSigRequiredSlices = [][]string{
{"spec", "signatures"},
}

var appProtectPolicyExtRefs = [][]string{
{"spec", "policy", "modificationsReference"},
{"spec", "policy", "blockingSettingReference"},
{"spec", "policy", "signatureSettingReference"},
{"spec", "policy", "serverTechnologyReference"},
{"spec", "policy", "headerReference"},
{"spec", "policy", "cookieReference"},
{"spec", "policy", "dataGuardReference"},
{"spec", "policy", "filetypeReference"},
{"spec", "policy", "methodReference"},
{"spec", "policy", "generalReference"},
{"spec", "policy", "parameterReference"},
{"spec", "policy", "sensitiveParameterReference"},
{"spec", "policy", "jsonProfileReference"},
{"spec", "policy", "xmlProfileReference"},
{"spec", "policy", "whitelistIpReference"},
{"spec", "policy", "responsePageReference"},
{"spec", "policy", "characterSetReference"},
{"spec", "policy", "cookieSettingsReference"},
{"spec", "policy", "headerSettingsReference"},
{"spec", "policy", "jsonValidationFileReference"},
{"spec", "policy", "xmlValidationFileReference"},
{"spec", "policy", "signatureSetReference"},
{"spec", "policy", "signatureReference"},
{"spec", "policy", "urlReference"},
{"spec", "policy", "threatCampaignReference"},
}

// ValidateAppProtectPolicy validates Policy resource
func ValidateAppProtectPolicy(policy *unstructured.Unstructured) error {
polName := policy.GetName()

err := ValidateRequiredFields(policy, appProtectPolicyRequiredFields)
if err != nil {
return fmt.Errorf("Error validating App Protect Policy %v: %w", polName, err)
return fmt.Errorf("error validating App Protect Policy %v: %w", polName, err)
}

extRefs, err := checkForExtRefs(policy)
if err != nil {
return fmt.Errorf("error validating App Protect Policy %v: %w", polName, err)
}

if len(extRefs) > 0 {
for _, ref := range extRefs {
glog.V(2).Infof("Warning: Field %s (External reference) is Deprecated.", ref)
}
}

return nil
Expand All @@ -36,7 +77,7 @@ func ValidateAppProtectLogConf(logConf *unstructured.Unstructured) error {
lcName := logConf.GetName()
err := ValidateRequiredFields(logConf, appProtectLogConfRequiredFields)
if err != nil {
return fmt.Errorf("Error validating App Protect Log Configuration %v: %w", lcName, err)
return fmt.Errorf("error validating App Protect Log Configuration %v: %w", lcName, err)
}

return nil
Expand All @@ -47,8 +88,23 @@ func ValidateAppProtectUserSig(userSig *unstructured.Unstructured) error {
sigName := userSig.GetName()
err := ValidateRequiredSlices(userSig, appProtectUserSigRequiredSlices)
if err != nil {
return fmt.Errorf("Error validating App Protect User Signature %v: %w", sigName, err)
return fmt.Errorf("error validating App Protect User Signature %v: %w", sigName, err)
}

return nil
}

func checkForExtRefs(policy *unstructured.Unstructured) ([]string, error) {
polName := policy.GetName()
out := []string{}
for _, ref := range appProtectPolicyExtRefs {
_, found, err := unstructured.NestedFieldNoCopy(policy.Object, ref...)
if err != nil {
return out, fmt.Errorf("error validating App Protect Policy %v: %w", polName, err)
}
if found {
out = append(out, strings.Join(ref, "."))
}
}
return out, nil
}
Loading

0 comments on commit f49b9a1

Please sign in to comment.