Skip to content

Commit

Permalink
Change enable-snippet to allow-snippet-annotation
Browse files Browse the repository at this point in the history
Signed-off-by: Ricardo Pchevuzinske Katz <rkatz@vmware.com>
  • Loading branch information
rikatz committed Sep 20, 2021
1 parent c6bf056 commit b5d8fa2
Show file tree
Hide file tree
Showing 12 changed files with 30 additions and 28 deletions.
2 changes: 1 addition & 1 deletion charts/ingress-nginx/ci/daemonset-customconfig-values.yaml
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
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
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
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
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
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
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
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
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
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
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
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

0 comments on commit b5d8fa2

Please sign in to comment.