(bugfix): OCPBUGS-3072 - fix `operator-sdk run bundle(-upgrade)` PSA related issues by everettraven · Pull Request #6210 · operator-framework/operator-sdk

everettraven · 2022-12-02T19:12:12Z

Description of the change:

Update default channel name selection to be different based on using FBC vs SQLite image.
Update the SecurityContext from the registry pod containers created by operator-sdk run bundle(-upgrade) to only be added to the containers when --security-context-config=restricted
Change default of --security-context-config flag to be legacy in anticipation of some OpenShift PSA changes.
Set the CatalogSource.grpcPodConfig.securityContextConfig to the same value of the --security-context-config flag.

Motivation for the change:

Fixes a bug where SQLite bundle images couldn't be properly run due to setting the default channel name to operator-sdk-run-bundle.
- This became a problem because we can manipulate FBCs to utilize this default channel, but we can not as easily manipulate the SQLite bundle to do the same.
Fixes a bug where even when specifying --security-context-config=legacy , the registry pod created still set the SecurityContext on the container as if it was in a restricted environment.

Checklist

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

Add a new changelog fragment in changelog/fragments (see changelog/fragments/00-template.yaml)
Add or update relevant sections of the docs website in website/content/en/docs

joelanford · 2022-12-02T20:26:58Z

@@ -139,7 +139,7 @@ func (c *IndexImageCatalogCreator) BindFlags(fs *pflag.FlagSet) {
 		"while pulling bundles")

 	// default to Restricted


Suggested change

// default to Restricted

// default to Legacy

Should be resolved as of 1d2e1ed

joelanford · 2022-12-02T20:30:27Z

+func withGrpcPodConfig(securityContextConfig string) func(*v1alpha1.CatalogSource) {
+	return func(cs *v1alpha1.CatalogSource) {
+		cs.Spec.GrpcPodConfig = &v1alpha1.GrpcPodConfig{
+			SecurityContextConfig: v1alpha1.SecurityConfig(securityContextConfig),
+		}
+	}
+}


Function name should align with what we're passing into the function. Either pass in the full grpcPodConfig or rename (and refactor) to just handle the securityContextConfig.

Suggestion showing the latter option:

Suggested change

func withGrpcPodConfig(securityContextConfig string) func(*v1alpha1.CatalogSource) {

return func(cs *v1alpha1.CatalogSource) {

cs.Spec.GrpcPodConfig = &v1alpha1.GrpcPodConfig{

SecurityContextConfig: v1alpha1.SecurityConfig(securityContextConfig),

}

}

}

func withGrpcPodSecurityContextConfig(securityContextConfig string) func(*v1alpha1.CatalogSource) {

return func(cs *v1alpha1.CatalogSource) {

if cs.Spec.GrpcPodConfig == nil {

cs.Spec.GrpcPodConfig = &v1alpha1.GrpcPodConfig{}

}

cs.Spec.GrpcPodConfig.SecurityContextConfig = v1alpha1.SecurityConfig(securityContextConfig)

}

}

}

Should be resolved as of 1d2e1ed

jmrodri · 2022-12-07T18:20:46Z

Fixes a bug where even when specifying --security-context-config=legacy , the registry pod created still set the SecurityContext on the container as if it was in a restricted environment.

@everettraven The SecurityContext was always there even before PSA. So are we saying that if the context is legacy mode that ALL of the security contexts are removed? It might be correct, but I'd make sure to ask that we're not undoing something that was needed before.

everettraven · 2022-12-19T19:43:50Z

The SecurityContext was always there even before PSA. So are we saying that if the context is legacy mode that ALL of the security contexts are removed? It might be correct, but I'd make sure to ask that we're not undoing something that was needed before.

@jmrodri I don't think we ran into this issue previously because we were by default using a container in the pod that wasn't running as root. The problem is that when you specify a specific index image that may be attempting to run as root and set --security-context-config=legacy we were still enforcing PSA in a way that made it so the container is not able to run as root, hence it would fail to run the registry pod.

I haven't taken a look at this for a couple weeks since I was on vacation, so I don't recall if I have tested all possible scenarios. Since I have to fix the failing unit tests I will run through a manual suite of tests again to ensure this is functioning as expected.

Signed-off-by: Bryce Palmer <bpalmer@redhat.com>

oceanc80 · 2022-12-20T20:42:51Z

- click `Edit` on the `v.*` branch rule.
- In section `Protect matching branches` of the `Rule settings` box, set "Required approving reviewers" to `6`.
+1. Lock down the `v1.3.x` branch to prevent further commits before the release completes:
+  1. Go to `Settings -> Branches` in the SDK repo.


Nit: I think the formatting here didn't quite do what you intended with the indents

Yeah, running the docs site locally it doesn't look like it formatted correctly. That being said, I pretty much snatched this straight from an existing section for locking down the branch and just changed some values. The section I snatched from:

operator-sdk/website/content/en/docs/contribution-guidelines/releasing.md

Lines 77 to 80 in cbeec47

1. Lock down the `master` branch to prevent further commits before the release completes:

1. Go to `Settings -> Branches` in the SDK repo.

1. Under `Branch protection rules`, click `Edit` on the `master` branch rule.

1. In section `Protect matching branches` of the `Rule settings` box, increase the number of required approving reviewers to 6.

oceanc80 · 2022-12-20T20:46:18Z

/lgtm

everettraven · 2022-12-22T14:56:29Z

/cherry-pick v1.26.x

openshift-cherrypick-robot · 2022-12-22T14:57:14Z

@everettraven: new pull request created: #6226

Details

In response to this:

/cherry-pick v1.26.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.

rashmigottipati · 2023-01-25T15:44:18Z

/cherry-pick v1.25.x

openshift-cherrypick-robot · 2023-01-25T15:45:01Z

@rashmigottipati: #6210 failed to apply on top of branch "v1.25.x":

Applying: (bugfix): OCPBUGS-3072 - fix run bundle PSA related issues
Using index info to reconstruct a base tree...
M	internal/olm/operator/registry/fbcindex/fbc_registry_pod.go
Falling back to patching base and 3-way merge...
Auto-merging internal/olm/operator/registry/fbcindex/fbc_registry_pod.go
CONFLICT (content): Merge conflict in internal/olm/operator/registry/fbcindex/fbc_registry_pod.go
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 (bugfix): OCPBUGS-3072 - fix run bundle PSA related issues
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

Details

In response to this:

/cherry-pick v1.25.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.

…related issues (operator-framework#6210) Signed-off-by: rashmigottipati <chowdary.grashmi@gmail.com>

…related issues (#6210) (#6261) Signed-off-by: rashmigottipati <chowdary.grashmi@gmail.com> Signed-off-by: rashmigottipati <chowdary.grashmi@gmail.com> Co-authored-by: Bryce Palmer <bpalmer@redhat.com>

operator-sdk run bundle(-upgrade) is running by default in legacy mode (see: operator-framework/operator-sdk#6210 ) which is not going to work out of the box on OCP/OKD 4.14 where PSA is enforced to restricted by default. Set --security-context-config=restricted to be able to execute on CI also on OCP/OKD 4.14. Signed-off-by: stirabos <stirabos@redhat.com>

openshift-ci Bot requested review from joelanford and theishshah December 2, 2022 19:12

everettraven temporarily deployed to deploy December 2, 2022 19:12 Inactive

grokspawn approved these changes Dec 2, 2022

View reviewed changes

joelanford reviewed Dec 2, 2022

View reviewed changes

everettraven temporarily deployed to deploy December 2, 2022 21:13 Inactive

everettraven temporarily deployed to deploy December 2, 2022 22:07 Inactive

everettraven temporarily deployed to deploy December 19, 2022 20:23 — with GitHub Actions Inactive

everettraven temporarily deployed to deploy December 20, 2022 18:54 — with GitHub Actions Inactive

fix another broken link

5cfa742

Signed-off-by: Bryce Palmer <bpalmer@redhat.com>

everettraven temporarily deployed to deploy December 20, 2022 19:07 — with GitHub Actions Inactive

oceanc80 reviewed Dec 20, 2022

View reviewed changes

openshift-ci Bot assigned oceanc80 Dec 20, 2022

openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Dec 20, 2022

everettraven merged commit 2058be2 into operator-framework:master Dec 22, 2022

everettraven deleted the bugfix/OCPBUGS3072-rbu branch December 22, 2022 14:52

openshift-cherrypick-robot mentioned this pull request Dec 22, 2022

[v1.26.x] (bugfix): OCPBUGS-3072 - fix operator-sdk run bundle(-upgrade) PSA related issues #6226

Merged

everettraven mentioned this pull request Jan 9, 2023

Add PSA Flag to Scorecard #6187

Merged

everettraven restored the bugfix/OCPBUGS3072-rbu branch January 25, 2023 17:47

rashmigottipati pushed a commit to rashmigottipati/operator-sdk that referenced this pull request Jan 25, 2023

(bugfix): OCPBUGS-3072 - fix operator-sdk run bundle(-upgrade) PSA …

878c2fe

…related issues (operator-framework#6210) Signed-off-by: rashmigottipati <chowdary.grashmi@gmail.com>

rashmigottipati mentioned this pull request Jan 25, 2023

[v1.25.x] (bugfix): OCPBUGS-3072 - fix operator-sdk run bundle(-upgrade) PSA related issues #6261

Merged

rashmigottipati pushed a commit to rashmigottipati/operator-sdk that referenced this pull request Jan 25, 2023

(bugfix): OCPBUGS-3072 - fix operator-sdk run bundle(-upgrade) PSA …

640d260

…related issues (operator-framework#6210) Signed-off-by: rashmigottipati <chowdary.grashmi@gmail.com>

rashmigottipati pushed a commit to rashmigottipati/operator-sdk that referenced this pull request Jan 25, 2023

(bugfix): OCPBUGS-3072 - fix operator-sdk run bundle(-upgrade) PSA …

1a9ee2a

…related issues (operator-framework#6210) Signed-off-by: rashmigottipati <chowdary.grashmi@gmail.com>

tiraboschi mentioned this pull request Jul 10, 2023

[test] Force --security-context-config=restricted on upgrade kubevirt/hyperconverged-cluster-operator#2428

Merged

11 tasks

		@@ -139,7 +139,7 @@ func (c IndexImageCatalogCreator) BindFlags(fs pflag.FlagSet) {
		"while pulling bundles")

		// default to Restricted

	1. Lock down the `master` branch to prevent further commits before the release completes:
	1. Go to `Settings -> Branches` in the SDK repo.
	1. Under `Branch protection rules`, click `Edit` on the `master` branch rule.
	1. In section `Protect matching branches` of the `Rule settings` box, increase the number of required approving reviewers to 6.

Conversation

everettraven commented Dec 2, 2022

Uh oh!

joelanford Dec 2, 2022

Choose a reason for hiding this comment

Uh oh!

everettraven Dec 2, 2022

Choose a reason for hiding this comment

Uh oh!

joelanford Dec 2, 2022

Choose a reason for hiding this comment

Uh oh!

everettraven Dec 2, 2022

Choose a reason for hiding this comment

Uh oh!

jmrodri commented Dec 7, 2022

Uh oh!

everettraven commented Dec 19, 2022

Uh oh!

oceanc80 Dec 20, 2022

Choose a reason for hiding this comment

Uh oh!

everettraven Dec 20, 2022

Choose a reason for hiding this comment

Uh oh!

oceanc80 commented Dec 20, 2022

Uh oh!

everettraven commented Dec 22, 2022

Uh oh!

openshift-cherrypick-robot commented Dec 22, 2022

Uh oh!

rashmigottipati commented Jan 25, 2023

Uh oh!

openshift-cherrypick-robot commented Jan 25, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants