Skip to content

Commit

Permalink
AP: Deprecate external refs (#2249)
Browse files Browse the repository at this point in the history
  • Loading branch information
rafwegv authored Mar 9, 2022
1 parent 3088395 commit 3b6b4a9
Show file tree
Hide file tree
Showing 6 changed files with 122 additions and 19 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/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
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
}
14 changes: 7 additions & 7 deletions pkg/apis/configuration/validation/appprotect_common.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,10 @@ func ValidateRequiredSlices(obj *unstructured.Unstructured, fieldsList [][]strin
for _, fields := range fieldsList {
field, found, err := unstructured.NestedSlice(obj.Object, fields...)
if err != nil {
return fmt.Errorf("Error checking for required field %v: %w", field, err)
return fmt.Errorf("error checking for required field %v: %w", field, err)
}
if !found {
return fmt.Errorf("Required field %v not found", field)
return fmt.Errorf("required field %v not found", field)
}
}
return nil
Expand All @@ -29,10 +29,10 @@ func ValidateRequiredFields(obj *unstructured.Unstructured, fieldsList [][]strin
for _, fields := range fieldsList {
field, found, err := unstructured.NestedMap(obj.Object, fields...)
if err != nil {
return fmt.Errorf("Error checking for required field %v: %w", field, err)
return fmt.Errorf("error checking for required field %v: %w", field, err)
}
if !found {
return fmt.Errorf("Required field %v not found", field)
return fmt.Errorf("required field %v not found", field)
}
}
return nil
Expand All @@ -46,7 +46,7 @@ var (

// ValidateAppProtectLogDestination validates destination for log configuration
func ValidateAppProtectLogDestination(dstAntn string) error {
errormsg := "Error parsing App Protect Log config: Destination must follow format: syslog:server=<ip-address | localhost>:<port> or fqdn or stderr or absolute path to file"
errormsg := "error parsing App Protect Log config: Destination must follow format: syslog:server=<ip-address | localhost>:<port> or fqdn or stderr or absolute path to file"
if !logDstEx.MatchString(dstAntn) {
return fmt.Errorf("%s Log Destination did not follow format", errormsg)
}
Expand All @@ -64,7 +64,7 @@ func ValidateAppProtectLogDestination(dstAntn string) error {
port, _ := strconv.Atoi(dstchunks[2])

if port > 65535 || port < 1 {
return fmt.Errorf("Error parsing port: %v not a valid port number", port)
return fmt.Errorf("error parsing port: %v not a valid port number", port)
}

ipstr := strings.Split(dstchunks[1], "=")[1]
Expand All @@ -77,7 +77,7 @@ func ValidateAppProtectLogDestination(dstAntn string) error {
}

if net.ParseIP(ipstr) == nil {
return fmt.Errorf("Error parsing host: %v is not a valid ip address or host name", ipstr)
return fmt.Errorf("error parsing host: %v is not a valid ip address or host name", ipstr)
}

return nil
Expand Down
45 changes: 45 additions & 0 deletions pkg/apis/configuration/validation/appprotect_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -174,3 +174,48 @@ func TestValidateAppProtectUserSig(t *testing.T) {
}
}
}

func TestCheckForExtRefs(t *testing.T) {
tests := []struct {
policy *unstructured.Unstructured
expectFound int
msg string
}{
{
policy: &unstructured.Unstructured{
Object: map[string]interface{}{
"spec": map[string]interface{}{
"policy": map[string]interface{}{
"signatures": []interface{}{},
},
},
},
},
expectFound: 0,
msg: "Policy with no refs",
},
{
policy: &unstructured.Unstructured{
Object: map[string]interface{}{
"spec": map[string]interface{}{
"policy": map[string]interface{}{
"jsonProfileReference": []interface{}{},
},
},
},
},
expectFound: 1,
msg: "Policy with refs",
},
}

for _, test := range tests {
refs, err := checkForExtRefs(test.policy)
if err != nil {
t.Errorf("Error in test case %s: function returned: %v", test.msg, err)
}
if len(refs) != test.expectFound {
t.Errorf("Error in test case %s: found %v expected: %v", test.msg, len(refs), test.expectFound)
}
}
}

0 comments on commit 3b6b4a9

Please sign in to comment.