Skip to content

OCPBUGS-1666: Expose flag to enable/disable PodSecurity#6062

Merged
jmrodri merged 7 commits into
operator-framework:masterfrom
jmrodri:run-bundle-bug
Oct 13, 2022
Merged

OCPBUGS-1666: Expose flag to enable/disable PodSecurity#6062
jmrodri merged 7 commits into
operator-framework:masterfrom
jmrodri:run-bundle-bug

Conversation

@jmrodri

@jmrodri jmrodri commented Oct 7, 2022

Copy link
Copy Markdown
Member

Description of the change:

Added --security-context-config flag to enable seccompprofile. It defaults to enabled to support k8s 1.25. You can disable it with --security-context-config=legacy

Signed-off-by: jesus m. rodriguez jesusr@redhat.com

Motivation for the change:
In k8s 1.25 PodSecurityAdmission is enabled. https://kubernetes.io/blog/2022/08/04/upcoming-changes-in-kubernetes-1-25/#podsecuritypolicy-removal

Fixes a few issues found downstream but affect upstream as well:

Checklist

If the pull request includes user-facing changes, extra documentation is required:

@openshift-ci openshift-ci Bot requested review from fabianvf and joelanford October 7, 2022 02:37
@jmrodri jmrodri temporarily deployed to deploy October 7, 2022 02:37 Inactive
@jmrodri jmrodri temporarily deployed to deploy October 7, 2022 02:37 Inactive
@jmrodri jmrodri temporarily deployed to deploy October 7, 2022 02:37 Inactive
@jmrodri jmrodri temporarily deployed to deploy October 7, 2022 02:37 Inactive
@jmrodri jmrodri temporarily deployed to deploy October 7, 2022 02:37 Inactive
@jmrodri jmrodri temporarily deployed to deploy October 7, 2022 02:37 Inactive
@jmrodri jmrodri temporarily deployed to deploy October 7, 2022 02:37 Inactive
@jmrodri jmrodri temporarily deployed to deploy October 7, 2022 02:42 Inactive
@jmrodri jmrodri temporarily deployed to deploy October 7, 2022 02:42 Inactive
@jmrodri jmrodri temporarily deployed to deploy October 7, 2022 02:45 Inactive
@jmrodri jmrodri temporarily deployed to deploy October 7, 2022 02:46 Inactive
@jmrodri

jmrodri commented Oct 7, 2022

Copy link
Copy Markdown
Member Author

Fixes a few issues upstream and downstream:

@jmrodri jmrodri temporarily deployed to deploy October 7, 2022 02:47 Inactive
@jmrodri jmrodri temporarily deployed to deploy October 7, 2022 02:47 Inactive
@jmrodri jmrodri temporarily deployed to deploy October 7, 2022 02:49 Inactive
@jmrodri jmrodri requested a review from everettraven October 7, 2022 02:54
@jmrodri

jmrodri commented Oct 7, 2022

Copy link
Copy Markdown
Member Author

If you pass in an unsupported option you get the help message:

$ operator-sdk run bundle quay.io/olmqe/upgradeindex-bundle:v0.1 --index-image quay.io/olmqe/largefbcindexwithupgradefbc:v4.11 --timeout 5m --security-context-config=fake -n jesusr
Error: invalid argument "fake" for "--security-context-config" flag: must be one of "legacy", or "restricted"
Usage:
  operator-sdk run bundle <bundle-image> [flags]

Flags:
      --ca-secret-name string                     Name of a generic secret containing a PEM root certificate file required to pull bundle images. This secret *must* be in the namespace that this command is configured to run in, and the file *must* be encoded under the key "cert.pem"
  -h, --help                                      help for bundle
      --index-image string                        index image in which to inject bundle (default "quay.io/operator-framework/opm:latest")
      --install-mode InstallModeValue             install mode
      --kubeconfig string                         Path to the kubeconfig file to use for CLI requests.
  -n, --namespace string                          If present, namespace scope for this CLI request
      --pull-secret-name string                   Name of image pull secret ("type: kubernetes.io/dockerconfigjson") required to pull bundle images. This secret *must* be both in the namespace and an imagePullSecret of the service account that this command is configured to run in
      --security-context-config SecurityContext   specifies the security context to use for the catalog pod
      --service-account string                    Service account name to bind registry objects to. If unset, the default service account is used. This value does not override the operator's service account
      --skip-tls                                  skip authentication of image registry TLS certificate when pulling a bundle image in-cluster
      --skip-tls-verify                           skip TLS certificate verification for container image registries while pulling bundles
      --timeout duration                          Duration to wait for the command to complete before failing (default 2m0s)
      --use-http                                  use plain HTTP for container image registries while pulling bundles

Global Flags:
      --plugins strings   plugin keys to be used for this subcommand execution
      --verbose           Enable verbose logging

FATA[0000] invalid argument "fake" for "--security-context-config" flag: must be one of "legacy", or "restricted" 

@jmrodri

jmrodri commented Oct 7, 2022

Copy link
Copy Markdown
Member Author

When restricted is enabled, the pod will have the seccompProfile set on it.

$ operator-sdk run bundle quay.io/olmqe/upgradeindex-bundle:v0.1 --index-image quay.io/olmqe/largefbcindexwithupgradefbc:v4.11 --timeout 5m --security-context-config=restricted -n jesusr

$ k get pod quay-io-olmqe-upgradeindex-bundle-v0-1 -o yaml -n jesusr | grep -i seccompProfile -A5 -B1
  securityContext:
    seccompProfile:
      type: RuntimeDefault
  serviceAccount: default
  serviceAccountName: default
  terminationGracePeriodSeconds: 30
  tolerations:

@jmrodri

jmrodri commented Oct 7, 2022

Copy link
Copy Markdown
Member Author

if legacy is enabled then no seccompprofile is added to the Pod.

$ operator-sdk run bundle quay.io/olmqe/upgradeindex-bundle:v0.1 --index-image quay.io/olmqe/largefbcindexwithupgradefbc:v4.11 --timeout 5m --security-context-config=legacy -n jesusr

$ k get pod quay-io-olmqe-upgradeindex-bundle-v0-1 -o yaml -n jesusr | grep -i seccompProfile -A5 -B1

@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 7, 2022
Comment thread website/content/en/docs/cli/operator-sdk_run_bundle-upgrade.md Outdated
Comment thread website/content/en/docs/cli/operator-sdk_run_bundle-upgrade.md Outdated
Comment thread website/content/en/docs/cli/operator-sdk_run_bundle.md Outdated

@everettraven everettraven left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a teeny nit on the changelog. Also need to run make generate to update the CLI docs with the latest changes.

Comment thread changelog/fragments/enable-seccompprofile-run-bundle.yaml Outdated
Comment thread .cncf-maintainers Outdated
jmrodri and others added 2 commits October 7, 2022 21:56
Added --security-context-config flag to enable seccompprofile.
It defaults to enabled to support k8s 1.25. You can disable it
with --security-context-config=legacy

Signed-off-by: jesus m. rodriguez <jesusr@redhat.com>
* Ignoring error from Set call in test
* Update .cncf maintainers
* Update run bundle(-upgrade) CLI docs

Signed-off-by: jesus m. rodriguez <jmrodri@gmail.com>
@jmrodri jmrodri temporarily deployed to deploy October 12, 2022 13:45 Inactive
@jmrodri jmrodri temporarily deployed to deploy October 12, 2022 13:45 Inactive
@jmrodri jmrodri temporarily deployed to deploy October 12, 2022 13:45 Inactive
@jmrodri jmrodri temporarily deployed to deploy October 12, 2022 13:45 Inactive
@jmrodri jmrodri temporarily deployed to deploy October 12, 2022 13:45 Inactive
@jmrodri jmrodri temporarily deployed to deploy October 12, 2022 13:45 Inactive
@openshift-ci openshift-ci Bot removed the lgtm Indicates that a PR is ready to be merged. label Oct 12, 2022
This was being duplicated because we had it in the text but were not
setting the value to a default value. Once we set the value to the
default cobra realized this and would output "(default: restricted)".

So removing the manually entered text fixes the duplicate.

Signed-off-by: jesus m. rodriguez <jesusr@redhat.com>
@jmrodri jmrodri temporarily deployed to deploy October 12, 2022 14:16 Inactive
@jmrodri jmrodri temporarily deployed to deploy October 12, 2022 14:16 Inactive
@jmrodri jmrodri temporarily deployed to deploy October 12, 2022 14:16 Inactive
@jmrodri jmrodri temporarily deployed to deploy October 12, 2022 14:16 Inactive
@jmrodri jmrodri temporarily deployed to deploy October 12, 2022 14:16 Inactive
@jmrodri jmrodri temporarily deployed to deploy October 12, 2022 14:16 Inactive
@jmrodri jmrodri temporarily deployed to deploy October 12, 2022 14:16 Inactive

@everettraven everettraven left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Oct 12, 2022
@jmrodri jmrodri merged commit ead5efa into operator-framework:master Oct 13, 2022
@jmrodri

jmrodri commented Oct 13, 2022

Copy link
Copy Markdown
Member Author

/cherry-pick v1.24.x

@openshift-cherrypick-robot

Copy link
Copy Markdown

@jmrodri: new pull request created: #6080

Details

In response to this:

/cherry-pick v1.24.x

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

tiraboschi added a commit to tiraboschi/release that referenced this pull request Jul 11, 2023
…text

Add a parameter to let the users specify a
value for --security-context-config (legacy/restricted, default
restricted) to by used by the operator-sdk for its catalog pod.

See: operator-framework/operator-sdk#6062

Signed-off-by: Simone Tiraboschi <stirabos@redhat.com>
openshift-merge-robot pushed a commit to openshift/release that referenced this pull request Jul 11, 2023
…text (#41134)

Add a parameter to let the users specify a
value for --security-context-config (legacy/restricted, default
restricted) to by used by the operator-sdk for its catalog pod.

See: operator-framework/operator-sdk#6062

Signed-off-by: Simone Tiraboschi <stirabos@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants