Skip to content
This repository has been archived by the owner on May 24, 2023. It is now read-only.

Fix SCC resource to only affect KIC pods #65

Merged
merged 4 commits into from
Mar 3, 2021
Merged

Conversation

Dean-Coakley
Copy link
Contributor

Fixes: #27

Bug

More details in: #27

If a pod is created is the same namespace it binds to the operators SCC and tries to use the nginx user which is unavailable.

Proposed changes

Priority of 20 overrides the max priority of the default: AnyPid. This
leads to the SCC binding to all pods created in the same namespace
rather than just KIC pods.

Priority is now changed to the default value so if a new pod is created,
it takes the default SCC rather than the SCC of the operator.

Before fix

$ oc get pod -n my-nginx-ingress my-nginx-ingress-controller-7bcb66bd76-4h4pl -o template='{{index .metadata.annotations "openshift.io/scc"}}'
nginx-ingress-scc%

$ oc run nginx -n my-nginx-ingress --image=nginx --restart=Never -- sleep 3600
pod/nginx created

$ oc get pod -n my-nginx-ingress nginx4 -o template='{{index .metadata.annotations "openshift.io/scc"}}'
nginx-ingress-scc%

After fix

$ oc get pod -n my-nginx-ingress my-nginx-ingress-controller-7bcb66bd76-4h4pl -o template='{{index .metadata.annotations "openshift.io/scc"}}'
nginx-ingress-scc%

$ oc run nginx -n my-nginx-ingress --image=nginx --restart=Never -- sleep 3600
pod/nginx created

$ oc get pod -n my-nginx-ingress nginx -o template='{{index .metadata.annotations "openshift.io/scc"}}'
anyuid%

Checklist

Before creating a PR, run through this checklist and mark each as complete.

  • I have read the CONTRIBUTING doc
  • I have checked that all unit tests pass after adding my changes
  • I have updated necessary documentation
  • I have rebased my branch onto master
  • I will ensure my PR is targeting the master branch and pulling from my branch from my own fork

Priority of 20 overrides the max priority of the default: AnyPid. This
leads to the SCC binding to all pods created in the same namespace
rather than just KIC pods.

Priority is now changed to the default value so if a new pod is created,
it takes the default SCC rather than the SCC of the operator.
@Dean-Coakley Dean-Coakley added the bug An issue reporting a potential bug label Feb 24, 2021
@Dean-Coakley Dean-Coakley self-assigned this Feb 24, 2021
@OchiengEd
Copy link

LGTM @Dean-Coakley . Not setting the priority allows Openshift to select the order to apply the SCCs from most restrictive to least restrictive if they have the same priority which when unset is 0. This eliminates conflict when used alongside stock SCCs with predefined priorities.

The Nginx Ingress controller needs both anyuid and nginx-ingress-scc SCCs.

There should not be a change from a security perspective. Since the same SCCs are used before and after.

@pleshakov
Copy link
Contributor

Hi @OchiengEd

Thanks for the help!

Does it make sense to keep the group field set?

Groups: []string{"system:authenticated"},
? I wonder if that could also cause any problems

@Dean-Coakley Dean-Coakley merged commit 56c5e69 into master Mar 3, 2021
@Dean-Coakley Dean-Coakley deleted the fix/scc-sa branch March 3, 2021 12:01
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug An issue reporting a potential bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

NGINX ingress operator creates SCC: nginx-ingress-scc and sets groups: system:authenticated
4 participants