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

Add a seccompProfile to the pod #3156

Merged

Conversation

shanman190
Copy link
Contributor

Is there a related GitHub Issue?

N/A

What is this change about?

Presently Korifi places a seccompProfile on the container, while this works for the container when you want to layer in a service mesh such as Istio you run into a few issues. During the Korifi installation, if you follow the install guide as described it has you place a pod-security.kubernetes.io/enforce=restricted on the root namespace, then this cascades to created organization and space namespaces. When this happens, for a space that desires to enable the Istio service mesh, the istio-init and istio-validation sidecar containers are started with the default seccompProfile which results in the entire pod being unable to be scheduled due to two containers wanting additional privileges. Istio has support for it's own custom, chained CNI which removes the need for the additional privileges in these more secure environments, but during it's mutation it strictly observes a seccompProfile placed upon the pod itself.

Does this PR introduce a breaking change?

No breaking changes.

Acceptance Steps

Bring up a Korifi environment utilizing Istio with pod-security.kubernetes.io/enforce=restricted on the space namespaces and the API Gateway integration.
profile.yaml

apiVersion: install.istio.io/v1alpha1
kind: IstioOperator
spec:
  profile: minimal
  components:
    cni:
      enabled: true
  meshConfig:
    accessLogFile: /dev/stdout
    outboundTrafficPolicy:
      mode: ALLOW_ANY
  values:
    cni:
      excludeNamespaces:
        - istio-system
        - kube-system
        - kpack
      logLevel: debug
    pilot:
      env:
        PILOT_ENABLE_ALPHA_GATEWAY_API: true
istioctl install --file profile.yaml

Tag your pair, your PM, and/or team

N/A

Additional Information

I found the reference to putting the seccompProfile on the pod in the Istio project's issue comments. You can find the specific comment here: istio/istio#35894 (comment). I've also verified upstream that kpack is already placing a seccompProfile on both the pod and the build container.

Copy link

linux-foundation-easycla bot commented Mar 7, 2024

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: shanman190 / name: Shannon Pamperl (e9b6cc0)

danail-branekov
danail-branekov previously approved these changes Mar 8, 2024
@danail-branekov danail-branekov enabled auto-merge (rebase) March 8, 2024 10:11
@danail-branekov
Copy link
Member

Thank you for the PR, @shanman190!

That is quite interesting, we were not aware that istio sidecars could run in restricted namespaces, good to know!

@georgethebeatle
Copy link
Member

Hey @shanman190 looks like there is one more test that needs to be adapted: https://github.com/cloudfoundry/korifi/actions/runs/8191034479/job/22431280462?pr=3156

Can you please fix this, so that we can merge your change?

auto-merge was automatically disabled March 8, 2024 14:26

Head branch was pushed to by a user without write access

@shanman190
Copy link
Contributor Author

The failing test has now been fixed. Thanks @danail-branekov and @georgethebeatle!

danail-branekov
danail-branekov previously approved these changes Mar 8, 2024
@shanman190
Copy link
Contributor Author

Ahh, ok, I see now that my other new test isn't actually valid and is replaced by the other integration test that had failed. I'll remove the current failing test and that should get this PR fixed up finally.

@danail-branekov
Copy link
Member

Ahh, ok, I see now that my other new test isn't actually valid and is replaced by the other integration test that had failed. I'll remove the current failing test and that should get this PR fixed up finally.

Thanks! Could you also take the opportunity to squash all the commits into one?

@shanman190
Copy link
Contributor Author

Will do!

@shanman190
Copy link
Contributor Author

Squashed and pushed. Things should be good now. Apologies about the couple of test bugs.

@danail-branekov danail-branekov enabled auto-merge (rebase) March 8, 2024 15:26
@danail-branekov danail-branekov merged commit 36f7f69 into cloudfoundry:main Mar 8, 2024
11 checks passed
@shanman190 shanman190 deleted the fix/pod-security-context branch March 8, 2024 23:00
danail-branekov added a commit that referenced this pull request Mar 12, 2024
PR #3156 changes the
definition of statefulset/job by setting the seccomp profile on the
statefulset/job template spec. This causes all workload pods to get
automatically restarted when upgrading from previous Korifi releases.

We introduce new `statefulsetRunnerTemporarySetPodSeccompProfile` and
`jobTaskRunnerTemporarySetPodSeccompProfile` helm values whose default
value would prevent altering existing statefulsets and jobs.

Those will be probably removed in future releases.

Co-authored-by: Danail Branekov <danailster@gmail.com>
danail-branekov added a commit that referenced this pull request Mar 12, 2024
PR #3156 changes the
definition of statefulset/job by setting the seccomp profile on the
statefulset/job template spec. This causes all workload pods to get
automatically restarted when upgrading from previous Korifi releases.

We introduce new `statefulsetRunnerTemporarySetPodSeccompProfile` and
`jobTaskRunnerTemporarySetPodSeccompProfile` helm values whose default
value would prevent altering existing statefulsets and jobs.

Those will be probably removed in future releases.

Co-authored-by: Danail Branekov <danailster@gmail.com>
marsteg pushed a commit to marsteg/korifi that referenced this pull request Mar 19, 2024
PR cloudfoundry#3156 changes the
definition of statefulset/job by setting the seccomp profile on the
statefulset/job template spec. This causes all workload pods to get
automatically restarted when upgrading from previous Korifi releases.

We introduce new `statefulsetRunnerTemporarySetPodSeccompProfile` and
`jobTaskRunnerTemporarySetPodSeccompProfile` helm values whose default
value would prevent altering existing statefulsets and jobs.

Those will be probably removed in future releases.

Co-authored-by: Danail Branekov <danailster@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants