Skip to content

CNTRLPLANE-1544: bindata: Use restricted-v3 scc for deployments - #1066

Open
tchap wants to merge 2 commits into
openshift:mainfrom
tchap:userns-operand
Open

CNTRLPLANE-1544: bindata: Use restricted-v3 scc for deployments#1066
tchap wants to merge 2 commits into
openshift:mainfrom
tchap:userns-operand

Conversation

@tchap

@tchap tchap commented Nov 11, 2025

Copy link
Copy Markdown

This effectively enforces user namespaces.

Summary by CodeRabbit

  • Security

    • Updated console and downloads deployments to use the newer restricted security policy.
    • Disabled host user access for improved pod isolation.
  • Tests

    • Updated deployment validation to reflect the strengthened security settings.

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Nov 11, 2025
@openshift-ci-robot

openshift-ci-robot commented Nov 11, 2025

Copy link
Copy Markdown
Contributor

@tchap: This pull request references CNTRLPLANE-1544 which is a valid jira issue.

Details

In response to this:

This effectively enforces user namespaces.

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 openshift-eng/jira-lifecycle-plugin repository.

This effectively enforces user namespaces.
@tchap

tchap commented Nov 12, 2025

Copy link
Copy Markdown
Author

/retest

1 similar comment
@tchap

tchap commented Nov 13, 2025

Copy link
Copy Markdown
Author

/retest

@openshift-ci

openshift-ci Bot commented Nov 13, 2025

Copy link
Copy Markdown
Contributor

@tchap: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e-aws-console f72a656 link true /test e2e-aws-console

Full PR test history. Your PR dashboard.

Details

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-sigs/prow repository. I understand the commands that are listed here.

@jhadvig jhadvig left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

/lgtm
/approve

@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Jan 14, 2026
@openshift-ci

openshift-ci Bot commented Jan 14, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jhadvig, tchap

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 14, 2026
@openshift-bot

Copy link
Copy Markdown
Contributor

Issues go stale after 90d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle stale

@openshift-ci openshift-ci Bot added lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Apr 15, 2026
@openshift-ci openshift-ci Bot removed the lgtm Indicates that a PR is ready to be merged. label Aug 19, 2026
@openshift-ci

openshift-ci Bot commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

New changes are detected. LGTM label has been removed.

@jhadvig jhadvig removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Aug 19, 2026
@openshift-ci openshift-ci Bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 19, 2026
@coderabbitai

coderabbitai Bot commented Aug 19, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: cf63495b-5b53-43b1-baf2-fd46d12721f6

📥 Commits

Reviewing files that changed from the base of the PR and between 61f7801 and 828b6b1.

📒 Files selected for processing (3)
  • bindata/assets/deployments/console-deployment.yaml
  • bindata/assets/deployments/downloads-deployment.yaml
  • pkg/console/subresource/deployment/deployment_test.go
🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

  • openshift/console (manual)

Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review.

📜 Recent review details
🧰 Additional context used
📓 Path-based instructions (11)
bindata/assets/**/*.yaml

📄 CodeRabbit inference engine (ARCHITECTURE.md)

Store static YAML assets in bindata/assets/ organized by resource type (configmaps/, deployments/, pdb/, routes/, services/)

Files:

  • bindata/assets/deployments/downloads-deployment.yaml
  • bindata/assets/deployments/console-deployment.yaml
**/*.{yaml,yml,json}

📄 CodeRabbit inference engine (Custom checks)

**/*.{yaml,yml,json}: Flag privileged: true, hostPID, hostNetwork, hostIPC, SYS_ADMIN capability, running as root without justification, and allowPrivilegeEscalation: true in container/Kubernetes manifests
When deployment manifests, operator code, or controllers are added/modified, ensure they do not introduce scheduling constraints assuming standard HA topology (3+ control-plane nodes, dedicated workers). Flag: required pod anti-affinity with maxUnavailable: 0 (deadlocks on SNO/TNF/TNA), pod topology spread with DoNotSchedule and hostname key (breaks on SNO), replica counts derived from node count without topology awareness, nodeSelector/node affinity targeting control-plane nodes (fails on HyperShift), scheduling to all control-plane nodes equally without excluding arbiter nodes (TNA), assuming dedicated worker nodes exist (SNO/TNF), or PodDisruptionBudgets designed for 3+ nodes (TNF/TNA). Do not flag if change checks ControlPlaneTopology, node counts, or topology labels before applying constraints.

Files:

  • bindata/assets/deployments/downloads-deployment.yaml
  • bindata/assets/deployments/console-deployment.yaml
{manifests,bindata/assets,quickstarts,examples,profile-patches}/**/*.{yaml,yml}

📄 CodeRabbit inference engine (.claude/skills/manifest-review.md)

{manifests,bindata/assets,quickstarts,examples,profile-patches}/**/*.{yaml,yml}: Kubernetes manifests under manifests/, bindata/assets/, quickstarts/, examples/, and profile-patches/ must include the appropriate cluster profile annotations when they are CVO-deployed resources (for example include.release.openshift.io/hypershift, include.release.openshift.io/ibm-cloud-managed, include.release.openshift.io/self-managed-high-availability, and include.release.openshift.io/single-node-developer).
Console resources in the manifest trees must include the capability annotation capability.openshift.io/name: Console.
RBAC manifests must follow least-privilege design: grant only the permissions needed, avoid wildcards unless truly justified, use verbs that match the actual operation set, and set correct apiGroups ("" for core APIs and specific groups for CRDs).
Resources must use the correct namespace for their role: openshift-console for console workload resources and openshift-console-operator for operator resources; any cross-namespace reference must be explicit and intentional.
YAML manifests should use 2-space indentation, keep fields in a consistent order, and include --- separators between multiple resources in the same file.
When binding roles to service accounts, use console-operator in the openshift-console-operator namespace for operator cross-namespace access, and use console in the openshift-console namespace for console workload-scoped access.

Files:

  • bindata/assets/deployments/downloads-deployment.yaml
  • bindata/assets/deployments/console-deployment.yaml
**/*.yaml

⚙️ CodeRabbit configuration file

**/*.yaml: Review YAML manifests based on content and kind.

Refer to /manifest-review when YAML contains:

  • kind: Role or kind: ClusterRole (RBAC review)
  • kind: RoleBinding or kind: ClusterRoleBinding
  • annotations: section (check for cluster profiles)
  • verbs: ["*"] or wildcard permissions
  • apiGroups: ["*"] or overly broad permissions
  • ServiceAccount references in subjects

Check for required annotations in manifests/:

  • include.release.openshift.io/hypershift
  • include.release.openshift.io/ibm-cloud-managed
  • include.release.openshift.io/self-managed-high-availability
  • include.release.openshift.io/single-node-developer
  • capability.openshift.io/name: Console

For quickstarts/, additionally check:

  • QuickStart spec structure
  • Task descriptions and prerequisites
  • See quickstarts/README.md for guidelines

Files:

  • bindata/assets/deployments/downloads-deployment.yaml
  • bindata/assets/deployments/console-deployment.yaml
**/*.{yaml,yml}

⚙️ CodeRabbit configuration file

**/*.{yaml,yml}: If this is a Kubernetes/OpenShift manifest or Helm template:

  • securityContext: runAsNonRoot, readOnlyRootFilesystem,
    allowPrivilegeEscalation: false
  • Drop ALL capabilities, add only what is required
  • Resource limits (cpu, memory) on every container
  • No hostPID, hostNetwork, hostIPC, privileged: true
  • NetworkPolicy defined for the namespace
  • OpenShift: SCC must be restricted or custom-scoped
  • Liveness + readiness probes defined
  • automountServiceAccountToken: false unless needed
  • RBAC: least privilege; no cluster-admin for workloads
  • Helm: no .Values interpolation in shell commands

Files:

  • bindata/assets/deployments/downloads-deployment.yaml
  • bindata/assets/deployments/console-deployment.yaml
**/*.go

📄 CodeRabbit inference engine (AGENTS.md)

**/*.go: Follow Go coding standards and patterns documented in CONVENTIONS.md
Organize imports according to conventions documented in CONVENTIONS.md
Use gofmt to format Go code with standard formatting
Run go vet checks on all Go packages

Follow Go coding standards and patterns as documented in CONVENTIONS.md, including proper import organization

Organize Go code following the repository structure: main entry point in cmd/console/main.go, API constants in pkg/api/, operator command setup in pkg/cmd/operator/, and version command in pkg/cmd/version/

**/*.go: Use gofmt for formatting Go code
Follow standard Go naming conventions
Group imports in order: standard lib, 3rd party, kube/openshift, internal (marked with comments)
Use meaningful error messages with context in Go code
Set status conditions using status.Handle* functions with type prefixes (*Degraded, *Progressing, *Available, *Upgradeable)
Use typed errors and wrap errors to preserve stack context

Flag MD5, SHA1, DES, RC4, 3DES, Blowfish, and ECB mode cryptographic usage. Also flag custom crypto implementations and non-constant-time comparison of secrets or tokens.

**/*.go: Do not use deprecated Go APIs such as ioutil.ReadFile, ioutil.WriteFile, ioutil.ReadAll, or net.Dial in Dial callbacks; use os.ReadFile, os.WriteFile, io.ReadAll, and DialContext instead.
When returning errors in Go, wrap them with %w and include meaningful context instead of returning the raw error or using %v.
Use specific error checks such as apierrors.IsNotFound(err) instead of matching error strings with strings.Contains(err.Error(), ...).
Propagate the caller’s context.Context through operations and avoid replacing it with context.Background() inside request/controller code.
Use defer to release acquired resources so cleanup happens on all return paths.
Avoid god functions: keep Go functions to roughly under 100 lines and split code with too many responsibilities into smaller...

Files:

  • pkg/console/subresource/deployment/deployment_test.go

⚙️ CodeRabbit configuration file

**/*.go: Review Go code following OpenShift operator patterns.
See CONVENTIONS.md for coding standards and patterns.

Refer to the following skills based on CODE PATTERNS, not just file paths:

Refer to /controller-review when code contains:

  • Controller struct types (e.g., type *Controller struct)
  • func New*Controller( factory functions
  • factory.New().WithFilteredEventsInformers( pattern
  • .ToController( method calls
  • Sync(ctx context.Context, controllerContext factory.SyncContext) methods
  • operatorConfig.Spec.ManagementState checks
  • status.NewStatusHandler or status.Handle* functions

Refer to /sync-handler-review when code contains:

  • Main operator sync functions (e.g., sync_v400.go content)
  • Sequential resource syncing with early returns
  • Incremental reconciliation loops
  • Multiple resourceapply.Apply*() calls in sequence
  • Dependency ordering of ConfigMaps → Secrets → Service Accounts → RBAC → Services → Deployments → Routes
  • Feature gate conditional logic

Refer to /go-quality-review for all Go code to check:

  • Deprecated imports: ioutil.ReadFile, ioutil.WriteFile, ioutil.ReadAll
  • Deprecated patterns: Dial without DialContext
  • Error handling: missing %w in fmt.Errorf
  • Code smells: deep nesting (4+ levels), functions >100 lines
  • Magic values: unexplained numbers/strings
  • Context propagation: context.Background() instead of passed ctx
  • Missing godoc on exported functions

Files:

  • pkg/console/subresource/deployment/deployment_test.go
**/*_test.go

📄 CodeRabbit inference engine (AGENTS.md)

Follow testing patterns and commands documented in TESTING.md

Follow testing patterns and commands as documented in TESTING.md, including running unit tests with 'make test-unit' and checks with 'make check'

**/*_test.go: Use table-driven tests for comprehensive coverage
Use httptest for HTTP handler testing in Go
Include proper cleanup functions in tests
Test both success and failure paths

In Go tests, do not ignore returned errors; check err and fail the test with t.Fatalf or t.Errorf as appropriate.

Files:

  • pkg/console/subresource/deployment/deployment_test.go

⚙️ CodeRabbit configuration file

**/*_test.go: Review test code for quality and patterns.

Refer to /unit-test-review when test is in pkg//*_test.go:**

  • Table-driven test structure with test cases
  • Use of go-test/deep for struct comparisons
  • Test naming conventions (TestFunctionName)
  • Error handling with wantErr pattern
  • Edge case coverage (nil, empty, boundary values)
  • Proper assertions with helpful error messages
  • Test isolation (no shared mutable state)

Refer to /e2e-test-review when test contains:

  • framework.MustNewClientset(t, nil) or similar e2e framework usage
  • wait.Poll or wait.PollImmediate patterns
  • retry.RetryOnConflict for updates
  • Cleanup via defer functions
  • Console/operator CR manipulations
  • Test assertions on cluster state

Suggest to use /e2e-test-review when:

  • PR adds new feature requiring e2e coverage
  • Test file is empty or skeleton
  • Comments indicate "TODO: add test"

Review for common issues:

  • Missing cleanup (defer statements)
  • Using time.Sleep instead of wait.Poll
  • Missing context timeouts
  • Vague error messages in assertions
  • Tests without table-driven structure when testing multiple cases
  • Ignoring errors with _
  • Tests without assertions

Files:

  • pkg/console/subresource/deployment/deployment_test.go
{pkg,cmd}/**/*.go

📄 CodeRabbit inference engine (CLAUDE.md)

Use gofmt for code formatting on pkg and cmd directories

{pkg,cmd}/**/*.go: Format code using gofmt -w ./pkg ./cmd
Run go vet checks on all Go packages in ./pkg and ./cmd

Files:

  • pkg/console/subresource/deployment/deployment_test.go
pkg/console/subresource/**/*.go

📄 CodeRabbit inference engine (ARCHITECTURE.md)

Use pkg/console/subresource/ packages for resource builders, with separate packages for each resource type (authentication, configmap, deployment, oauthclient, route, secret, etc.)

Files:

  • pkg/console/subresource/deployment/deployment_test.go
pkg/**/*_test.go

📄 CodeRabbit inference engine (.claude/skills/unit-test-review.md)

pkg/**/*_test.go: Most unit tests should use the table-driven test pattern, including a tests := []struct{...} table and t.Run(tt.name, ...) subtests for scenarios with multiple cases.
Test function names and subtest case names should be descriptive of the behavior or scenario being tested (for example, TestGetNodeComputeEnvironments or "Custom hostname and TLS secret set").
Use github.com/go-test/deep (deep.Equal) for struct comparisons instead of == or manual field-by-field checks.
Cover both success and failure paths in unit tests, including edge cases such as empty inputs, boundary values, missing fields, duplicates, and large inputs.
Structure tests using Arrange-Act-Assert so setup, execution, and verification are clearly separated.
When testing error-returning functions, assert error presence correctly and, when relevant, validate the error message substring instead of ignoring the error or discarding it with _.
Prefer dependency injection via interfaces for testability, and keep tests isolated so they do not depend on execution order or shared mutable state.
Extract repeated setup into helper functions when common test fixtures are reused across multiple tests.
Write specific, informative assertions that explain what failed instead of vague or silent failures.
Inline simple test data, but move complex fixtures to helper functions or testdata/ files.
Avoid tests that rely on execution order, share global mutable state, use hardcoded sleeps, omit assertions, or verify implementation details instead of behavior.

Files:

  • pkg/console/subresource/deployment/deployment_test.go
**/*.{py,js,ts,go,rs,java,rb,php,kt,swift,cs}

⚙️ CodeRabbit configuration file

**/*.{py,js,ts,go,rs,java,rb,php,kt,swift,cs}: Injection prevention (prodsec-skills):

  • SQL: parameterized queries only; no string concatenation
  • Command: no shell=True, os.system, or backtick exec with user input
  • LDAP/XPath: escape special characters in filters
  • Path traversal: canonicalize paths, reject ../
  • Deserialization: no pickle/yaml.load()/eval on untrusted data
  • Prototype pollution: no recursive merge of untrusted objects
  • Validate at trust boundaries with allow-lists, not deny-lists
  • Normalize Unicode and anchor regexes (^$); watch for ReDoS

Files:

  • pkg/console/subresource/deployment/deployment_test.go
🔇 Additional comments (3)
bindata/assets/deployments/console-deployment.yaml (1)

26-26: 🗄️ Data Integrity & Integration

Verify generated bindata output for both deployment assets.

The deployment builders consume bindata.MustAsset, so both source YAML changes require synchronized generated data.

  • bindata/assets/deployments/console-deployment.yaml#L26-L26: confirm restricted-v3 is present in generated bindata data.
  • bindata/assets/deployments/console-deployment.yaml#L35-L35: confirm hostUsers: false is present in generated bindata data.
  • bindata/assets/deployments/downloads-deployment.yaml#L27-L27: confirm restricted-v3 is present in generated bindata data.
  • bindata/assets/deployments/downloads-deployment.yaml#L35-L35: confirm hostUsers: false is present in generated bindata data.
pkg/console/subresource/deployment/deployment_test.go (2)

144-144: LGTM!

Also applies to: 254-254, 336-336, 418-418, 496-496


1762-1762: LGTM!

Also applies to: 1894-1894, 1939-1939


Walkthrough

The console and downloads deployments now require the restricted-v3 security context constraint and set hostUsers: false. Deployment test fixtures now match these settings.

Changes

Pod security context

Layer / File(s) Summary
Deployment security settings
bindata/assets/deployments/*-deployment.yaml
Console and downloads deployments now require restricted-v3 and set hostUsers: false.
Deployment test expectations
pkg/console/subresource/deployment/deployment_test.go
Console and downloads deployment fixtures now expect the updated SCC annotation and HostUsers: false.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: 🔵 Low · up to 828b6

The deployment manifests now enforce restricted-v3 and disable host users; before merging, the generated deployment output should be verified to contain both changes for console and downloads deployments. This is a bounded follow-up risk and is otherwise mergeable with owner awareness.

Suggested reviewers: spadgett, therealjon

🚥 Pre-merge checks | ✅ 14 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description contains only a brief outcome and omits the required analysis, solution, testing, browser, additional information, and reviewer sections. Complete the required template sections with root cause, solution details, test setup, test cases, browser status, additional information, and reviewers.
✅ Passed checks (14 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the Jira issue and the main change to use the restricted-v3 SCC for deployments.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Stable And Deterministic Test Names ✅ Passed The PR changes only deployment expectations and HostUsers fields; the affected test uses static standard Go Test functions and adds no Ginkgo titles or dynamic test names.
Test Structure And Quality ✅ Passed The changed file uses standard Go table tests, not Ginkgo. It adds in-memory deployment expectations only; no cluster resources, cleanup, waits, or missing timeout paths were introduced.
Microshift Test Compatibility ✅ Passed The PR adds no Ginkgo e2e tests; it only updates YAML and existing table-driven Go tests with HostUsers and SCC expectations.
Single Node Openshift (Sno) Test Compatibility ✅ Passed The diff adds no Ginkgo e2e tests. It only changes deployment manifests and existing standard-library Go unit-test expectations.
Topology-Aware Scheduling Compatibility ✅ Passed The diff changes only SCC annotations and adds hostUsers:false; it adds no topology or scheduling constraint. Existing topology-aware replicas, affinity, strategy, and external node-selector logic...
Ote Binary Stdout Contract ✅ Passed The PR changes only deployment SCC/hostUsers fields and test expectations. It adds no OTE binary or process-level stdout writes.
Ipv6 And Disconnected Network Test Compatibility ✅ Passed The PR changes only two deployment YAML files and Go unit tests; no new Ginkgo e2e tests, IPv4 assumptions, or external connectivity were introduced.
No-Weak-Crypto ✅ Passed The reviewed deployment and test changes contain no MD5, SHA1, DES, RC4, 3DES, Blowfish, ECB, custom crypto, or secret-comparison code.
Container-Privileges ✅ Passed The diff adds only restricted-v3 annotations and hostUsers: false; manifests retain runAsNonRoot: true and allowPrivilegeEscalation: false, with no forbidden privilege or host namespace settings.
No-Sensitive-Data-In-Logs ✅ Passed The PR adds only SCC annotations, hostUsers settings, and test expectations; the diff adds no logging calls or sensitive data.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@openshift-ci-robot

openshift-ci-robot commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

@tchap: This pull request references CNTRLPLANE-1544 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the epic to target either version "5.1.0." or "openshift-5.1.0.", but it targets "openshift-4.22" instead.

Details

In response to this:

This effectively enforces user namespaces.

Summary by CodeRabbit

  • Security

  • Updated console and downloads deployments to use the newer restricted security policy.

  • Disabled host user access for improved pod isolation.

  • Tests

  • Updated deployment validation to reflect the strengthened security settings.

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 openshift-eng/jira-lifecycle-plugin repository.

@jhadvig

jhadvig commented Aug 19, 2026

Copy link
Copy Markdown
Member

/hold

/payload-job periodic-ci-openshift-console-operator-main-e2e-hypershift-conformance

@openshift-ci

openshift-ci Bot commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

@jhadvig: trigger 0 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command

@openshift-ci openshift-ci Bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants