update default block list,docs, tests

[strongjz]

1.0.5 created a breaking change with a default blocklist for user, this PR reverts that and makes the default blocklist an empty string.

What this PR does / why we need it:

fixes #7939

Types of changes

• [ X ] Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Documentation only

How Has This Been Tested?

make test and e2e-test locally

Checklist:

• [X ] My change requires a change to the documentation.
• [X ] I have updated the documentation accordingly.
• [X ] I've read the CONTRIBUTION guide
• [X ] I have added tests to cover my changes.
• [X ] All new and existing tests passed.

[strongjz]

/ok-to-test

[strongjz]

/kind bug
/priority critical-urgent
/triage accepted

[strongjz]

/assign @rikatz

[rikatz]

/lgtm
I'm on mobile but just to confirm, we test the custom blocklist scenario right?

Thanks!!!

[rikatz]

/hold
Unhold as desired!

[strongjz]

I left the tests in there with a custom list, just added one with the no setting so the new default empty string.

[rikatz]

I'm checking here why the tests are failing

[rikatz]

https://github.com/kubernetes/ingress-nginx/pull/7874/files#diff-9dcbb8eba12c61d6f3dde188c0b8d8332365139e5062995d322795e02710fb6cR45 Probably being caused by this, happened locally...

strange as this was going just fine, will check it again with bigger timeout just to remove this as something that may happen

[rikatz]

In https://github.com/kubernetes/ingress-nginx/blob/main/internal/ingress/controller/store/store.go#L826-L830 you can remove/replace for the following block (I have left a klog.Errorf that should be removed as well...)

	if s.backendConfig.AnnotationValueWordBlocklist != "" {
		if err := checkBadAnnotationValue(copyIng.Annotations, s.backendConfig.AnnotationValueWordBlocklist); err != nil {
			klog.Errorf("skipping ingress %s: %s", key, err)
			return
		}
	}

In https://github.com/kubernetes/ingress-nginx/blob/main/internal/ingress/controller/controller.go#L245
add the same check like:

var arraybadWords []string
if s.backendConfig.AnnotationValueWordBlocklist != "" {
   arraybadWords = strings.Split(strings.TrimSpace(cfg.AnnotationValueWordBlocklist), ",")
}

[rikatz]

One more:

As we are turning the default value as empty string, on our tests (like /go/src/k8s.io/ingress-nginx/test/e2e/settings/badannotationvalues.go:65) you probably want to configure with the default values (or remove this test)

[strongjz]

/label tide/merge-method-squash

[strongjz]

/retest

[rikatz]

nit/followup: turn this into a warning (my bad, I left as error, people complained!)

[rikatz]

/lgtm
Thanks.

Left a small followup/cleanup to move the error into warning, but IMO we can merge this, just do the small fix later and release

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rikatz, strongjz

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [rikatz,strongjz]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[rikatz]

/hold cancel