Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AP: Deprecate external refs #2249

Merged
merged 4 commits into from
Mar 9, 2022
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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.

rafwegv marked this conversation as resolved.
Show resolved Hide resolved
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
}