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

Change enable-snippet to allow-snippet-annotation #7670

Merged
merged 1 commit into from
Sep 20, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion charts/ingress-nginx/ci/daemonset-customconfig-values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ controller:
tag: 1.0.0-dev
digest: null
kind: DaemonSet
enableSnippetDirectives: false
allowSnippetAnnotations: false
admissionWebhooks:
enabled: false
service:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ controller:
digest: null
config:
use-proxy-protocol: "true"
enableSnippetDirectives: false
allowSnippetAnnotations: false
admissionWebhooks:
enabled: false
service:
Expand Down
2 changes: 1 addition & 1 deletion charts/ingress-nginx/templates/controller-configmap.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ metadata:
name: {{ include "ingress-nginx.controller.fullname" . }}
namespace: {{ .Release.Namespace }}
data:
enable-snippet-directives: "{{ .Values.controller.enableSnippetDirectives }}"
allow-snippet-annotations: "{{ .Values.controller.allowSnippetAnnotations }}"
{{- if .Values.controller.addHeaders }}
add-headers: {{ .Release.Namespace }}/{{ include "ingress-nginx.fullname" . }}-custom-add-headers
{{- end }}
Expand Down
4 changes: 2 additions & 2 deletions charts/ingress-nginx/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -70,10 +70,10 @@ controller:
ingressClassByName: false

# This configuration defines if Ingress Controller should allow users to set
# their own *-snippet directives/annotations, otherwise this is forbidden / dropped
# their own *-snippet annotations, otherwise this is forbidden / dropped
# when users add those annotations.
# Global snippets in ConfigMap are still respected
enableSnippetDirectives: true
allowSnippetAnnotations: true

# Required for use with CNI based kubernetes installations (such as ones set up by kubeadm),
# since CNI and hostport don't mix yet. Can be deprecated once https://github.com/kubernetes/kubernetes/issues/23920
Expand Down
15 changes: 8 additions & 7 deletions docs/user-guide/nginx-configuration/configmap.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ The following table shows a configuration option's name, type, and the default v
|:---|:---|:------|
|[add-headers](#add-headers)|string|""|
|[allow-backend-server-header](#allow-backend-server-header)|bool|"false"|
|[allow-snippet-annotations](#allow-snippet-annotations)|bool|true|
|[hide-headers](#hide-headers)|string array|empty|
|[access-log-params](#access-log-params)|string|""|
|[access-log-path](#access-log-path)|string|"/var/log/nginx/access.log"|
Expand All @@ -46,7 +47,6 @@ The following table shows a configuration option's name, type, and the default v
|[disable-access-log](#disable-access-log)|bool|false|
|[disable-ipv6](#disable-ipv6)|bool|false|
|[disable-ipv6-dns](#disable-ipv6-dns)|bool|false|
|[enable-snippet-directives](#enable-snippet-directives)|bool|true|
|[enable-underscores-in-headers](#enable-underscores-in-headers)|bool|false|
|[enable-ocsp](#enable-ocsp)|bool|false|
|[ignore-invalid-headers](#ignore-invalid-headers)|bool|true|
Expand Down Expand Up @@ -214,6 +214,13 @@ Sets custom headers from named configmap before sending traffic to the client. S

Enables the return of the header Server from the backend instead of the generic nginx string. _**default:**_ is disabled

## allow-snippet-annotations

Enables Ingress to parse and add *-snippet annotations/directives created by the user. _**default:**_ `true`;

Warning: We recommend enabling this option only if you TRUST users with permission to create Ingress objects, as this
may allow a user to add restricted configurations to the final nginx.conf file

## hide-headers

Sets additional header that will not be passed from the upstream server to the client response.
Expand Down Expand Up @@ -317,12 +324,6 @@ Disable listening on IPV6. _**default:**_ `false`; IPv6 listening is enabled

Disable IPV6 for nginx DNS resolver. _**default:**_ `false`; IPv6 resolving enabled.

## enable-snippet-directives

Enables Ingress to parse and add *-snippet annotations/directives created by the user. _**default:**_ `true`;
Obs.: We recommend enabling this option only if you TRUST users with permission to create Ingress objects, as this
may allow a user to add restricted configurations to the final nginx.conf file

## enable-underscores-in-headers

Enables underscores in header names. _**default:**_ is disabled
Expand Down
7 changes: 4 additions & 3 deletions internal/ingress/controller/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,9 +93,9 @@ const (
type Configuration struct {
defaults.Backend `json:",squash"`

// EnableSnippetDirectives enable users to add their own snippets via ingress annotation.
// AllowSnippetAnnotations enable users to add their own snippets via ingress annotation.
// If disabled, only snippets added via ConfigMap are added to ingress.
EnableSnippetDirectives bool `json:"enable-snippet-directives"`
AllowSnippetAnnotations bool `json:"allow-snippet-annotations"`

// Sets the name of the configmap that contains the headers to pass to the client
AddHeaders string `json:"add-headers,omitempty"`
Expand Down Expand Up @@ -761,7 +761,8 @@ func NewDefault() Configuration {
defGlobalExternalAuth := GlobalExternalAuth{"", "", "", "", "", append(defResponseHeaders, ""), "", "", "", []string{}, map[string]string{}}

cfg := Configuration{
EnableSnippetDirectives: true,

AllowSnippetAnnotations: true,
AllowBackendServerHeader: false,
AccessLogPath: "/var/log/nginx/access.log",
AccessLogParams: "",
Expand Down
10 changes: 5 additions & 5 deletions internal/ingress/controller/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,7 @@ func (n *NGINXController) CheckIngress(ing *networking.Ingress) error {
}
}

if !cfg.EnableSnippetDirectives && strings.HasSuffix(key, "-snippet") {
if !cfg.AllowSnippetAnnotations && strings.HasSuffix(key, "-snippet") {
return fmt.Errorf("%s annotation cannot be used. Snippet directives are disabled by the Ingress administrator", key)
}

Expand Down Expand Up @@ -550,7 +550,7 @@ func (n *NGINXController) getBackendServers(ingresses []*ingress.Ingress) ([]*in
ingKey := k8s.MetaNamespaceKey(ing)
anns := ing.ParsedAnnotations

if !n.store.GetBackendConfiguration().EnableSnippetDirectives {
if !n.store.GetBackendConfiguration().AllowSnippetAnnotations {
dropSnippetDirectives(anns, ingKey)
}

Expand Down Expand Up @@ -830,7 +830,7 @@ func (n *NGINXController) createUpstreams(data []*ingress.Ingress, du *ingress.B
ingKey := k8s.MetaNamespaceKey(ing)
anns := ing.ParsedAnnotations

if !n.store.GetBackendConfiguration().EnableSnippetDirectives {
if !n.store.GetBackendConfiguration().AllowSnippetAnnotations {
dropSnippetDirectives(anns, ingKey)
}

Expand Down Expand Up @@ -1124,7 +1124,7 @@ func (n *NGINXController) createServers(data []*ingress.Ingress,
ingKey := k8s.MetaNamespaceKey(ing)
anns := ing.ParsedAnnotations

if !n.store.GetBackendConfiguration().EnableSnippetDirectives {
if !n.store.GetBackendConfiguration().AllowSnippetAnnotations {
dropSnippetDirectives(anns, ingKey)
}

Expand Down Expand Up @@ -1204,7 +1204,7 @@ func (n *NGINXController) createServers(data []*ingress.Ingress,
ingKey := k8s.MetaNamespaceKey(ing)
anns := ing.ParsedAnnotations

if !n.store.GetBackendConfiguration().EnableSnippetDirectives {
if !n.store.GetBackendConfiguration().AllowSnippetAnnotations {
dropSnippetDirectives(anns, ingKey)
}

Expand Down
4 changes: 2 additions & 2 deletions internal/ingress/controller/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,7 @@ func TestCheckIngress(t *testing.T) {
nginx.store = fakeIngressStore{
ingresses: []*ingress.Ingress{},
configuration: ngx_config.Configuration{
EnableSnippetDirectives: false,
AllowSnippetAnnotations: false,
},
}
nginx.command = testNginxTestCommand{
Expand Down Expand Up @@ -2309,7 +2309,7 @@ func TestGetBackendServers(t *testing.T) {
SelfLink: fmt.Sprintf("/api/v1/namespaces/%s/configmaps/config", ns),
},
Data: map[string]string{
"enable-snippet-directives": "false",
"allow-snippet-annotations": "false",
},
}
},
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/annotations/modsecurity/modsecurity.go
Original file line number Diff line number Diff line change
Expand Up @@ -316,7 +316,7 @@ var _ = framework.DescribeAnnotation("modsecurity owasp", func() {
f.SetNginxConfigMapData(map[string]string{
"enable-modsecurity": "true",
"enable-owasp-modsecurity-crs": "true",
"enable-snippet-directives": "false",
"allow-snippet-annotations": "false",
"modsecurity-snippet": expectedComment,
})

Expand Down
4 changes: 2 additions & 2 deletions test/e2e/annotations/serversnippet.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,10 +67,10 @@ var _ = framework.DescribeAnnotation("server-snippet", func() {
}

ing := framework.NewSingleIngress(host, "/", host, f.Namespace, framework.EchoService, 80, annotations)
f.UpdateNginxConfigMapData("enable-snippet-directives", "false")
f.UpdateNginxConfigMapData("allow-snippet-annotations", "false")
defer func() {
// Return to the original value
f.UpdateNginxConfigMapData("enable-snippet-directives", "true")
f.UpdateNginxConfigMapData("allow-snippet-annotations", "true")
}()
// Sleep a while just to guarantee that the configmap is applied
framework.Sleep()
Expand Down
4 changes: 2 additions & 2 deletions test/e2e/annotations/snippet.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,10 +63,10 @@ var _ = framework.DescribeAnnotation("configuration-snippet", func() {
}

ing := framework.NewSingleIngress(host, "/", host, f.Namespace, framework.EchoService, 80, annotations)
f.UpdateNginxConfigMapData("enable-snippet-directives", "false")
f.UpdateNginxConfigMapData("allow-snippet-annotations", "false")
defer func() {
// Return to the original value
f.UpdateNginxConfigMapData("enable-snippet-directives", "true")
f.UpdateNginxConfigMapData("allow-snippet-annotations", "true")
}()
// Sleep a while just to guarantee that the configmap is applied
framework.Sleep()
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/settings/server_snippet.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ var _ = framework.DescribeSetting("configmap server-snippet", func() {
hostAnnots := "serverannotssnippet2.foo.com"

f.SetNginxConfigMapData(map[string]string{
"enable-snippet-directives": "false",
"allow-snippet-annotations": "false",
"server-snippet": `
more_set_headers "Globalfoo: Foooo";`,
})
Expand Down