Skip to content

CONSOLE-5550: Add ObjectSrc to ConsolePlugin CSP directives - #3128

Closed
shahsahil264 wants to merge 3 commits into
openshift:masterfrom
shahsahil264:CONSOLE-5550
Closed

shahsahil264 wants to merge 3 commits into
openshift:masterfrom
shahsahil264:CONSOLE-5550

Conversation

@shahsahil264

Copy link
Copy Markdown

Analysis / Root cause:
ConsolePlugin CSP validation currently rejects ObjectSrc, preventing plugins from declaring sources for <object> and <embed> elements.

Solution description:
Add ObjectSrc to DirectiveType and its validation enum, document its behavior, and regenerate CRD, Swagger, and OpenAPI artifacts. Preserve the existing validation rules and directive limits.

MediaSrc, ChildSrc, and WorkerSrc are intentionally excluded from the plugin-facing API. The corresponding Console backend change hardcodes media-src 'none', child-src 'none', and worker-src 'self'. The worker exception preserves the existing same-origin load-test.sw.js service worker. These policies remain non-configurable through ConsolePlugin; extensibility can be considered later if a specific plugin needs it and requests it.

Screenshots / screen recording:
Not applicable: API/schema change.

Test setup:
Use the repository's local envtest API server; no cluster credentials are required. Full generation and verification ran in a disposable checkout at the import path required by the protobuf generator. The validated patch is byte-identical to the seven changed files in the working checkout.

Test cases:

  • make update: passed across all API groups; reproduced the existing patch exactly.
  • make test-unit: passed (8 packages containing tests)
  • make -C console/v1 test: 34 passed, 0 failed
  • make verify PULL_BASE_SHA=HEAD: passed, including both lint configurations (0 issues)
  • Acceptance coverage for ObjectSrc and rejection coverage for MediaSrc, duplicate ObjectSrc entries, and quoted ObjectSrc values.

Browser conformance:
Not applicable: API/schema change.

Additional info:
Jira: https://redhat.atlassian.net/browse/CONSOLE-5550

Coordinate rollout with the corresponding console-operator dependency update and Bridge support. Do not enable ObjectSrc declarations until the deployed Bridge recognizes them; remove declarations before rolling back to an older Bridge.

Reviewers and assignees:
Repository maintainers.

Signed-off-by: Sahil Shah <sahshah@redhat.com>
Allow plugins to declare object-src sources while preserving existing CSP validation and limits. Regenerate the schemas and document aggregation behavior, with validation coverage for acceptance and invalid declarations.

Signed-off-by: Sahil Shah <sahshah@redhat.com>
Integrate the earlier ObjectSrc branch without a force push. Keep the exact source and generated artifacts validated against current upstream.

Signed-off-by: Sahil Shah <sahshah@redhat.com>
@openshift-merge-bot

Copy link
Copy Markdown
Contributor

Pipeline controller notification

This PR uses the pipeline controller for second-stage tests. Selection and triggering follow the repository configuration.

Use /test ? to list jobs and /pipeline help for commands.

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

openshift-ci-robot commented Oct 26, 2026 •

Copy link
Copy Markdown

@shahsahil264: This pull request references CONSOLE-5550 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 story to target the "5.1.0" version, but no target version was set.

Details

In response to this:

Analysis / Root cause:
ConsolePlugin CSP validation currently rejects ObjectSrc, preventing plugins from declaring sources for <object> and <embed> elements.

Solution description:
Add ObjectSrc to DirectiveType and its validation enum, document its behavior, and regenerate CRD, Swagger, and OpenAPI artifacts. Preserve the existing validation rules and directive limits.

MediaSrc, ChildSrc, and WorkerSrc are intentionally excluded from the plugin-facing API. The corresponding Console backend change hardcodes media-src 'none', child-src 'none', and worker-src 'self'. The worker exception preserves the existing same-origin load-test.sw.js service worker. These policies remain non-configurable through ConsolePlugin; extensibility can be considered later if a specific plugin needs it and requests it.

Screenshots / screen recording:
Not applicable: API/schema change.

Test setup:
Use the repository's local envtest API server; no cluster credentials are required. Full generation and verification ran in a disposable checkout at the import path required by the protobuf generator. The validated patch is byte-identical to the seven changed files in the working checkout.

Test cases:

  • make update: passed across all API groups; reproduced the existing patch exactly.
  • make test-unit: passed (8 packages containing tests)
  • make -C console/v1 test: 34 passed, 0 failed
  • make verify PULL_BASE_SHA=HEAD: passed, including both lint configurations (0 issues)
  • Acceptance coverage for ObjectSrc and rejection coverage for MediaSrc, duplicate ObjectSrc entries, and quoted ObjectSrc values.

Browser conformance:
Not applicable: API/schema change.

Additional info:
Jira: https://redhat.atlassian.net/browse/CONSOLE-5550

Coordinate rollout with the corresponding console-operator dependency update and Bridge support. Do not enable ObjectSrc declarations until the deployed Bridge recognizes them; remove declarations before rolling back to an older Bridge.

Reviewers and assignees:
Repository maintainers.

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.

@coderabbitai

coderabbitai Bot commented Oct 26, 2026 •

Copy link
Copy Markdown

No actionable comments were generated in the recent review.

Recent review info
Run configuration
  • Source mode: Auto
  • Repository: openshift/api
  • Branch: master
  • File: .coderabbit.yaml
  • Commit: 9809689
  • Layers: Repository YAML (base), Central YAML (inherited)
  • Review profile: CHILL
  • Plan: Enterprise
  • Run ID: bd5eb5fc-5733-43b6-80b1-2fc2656081b9
Commits

Reviewing files that changed from the base of the PR and between 9809689 and b70b941.

Files ignored due to path filters: 5
  • console/v1/zz_generated.crd-manifests/90_consoleplugins.crd.yaml is excluded by !**/zz_generated.crd-manifests/*
  • console/v1/zz_generated.featuregated-crd-manifests/consoleplugins.console.openshift.io/AAA_ungated.yaml is excluded by !**/zz_generated.featuregated-crd-manifests/**
  • console/v1/zz_generated.swagger_doc_generated.go is excluded by !**/zz_generated*
  • openapi/generated_openapi/zz_generated.openapi.go is excluded by !openapi/**, !**/zz_generated*
  • openapi/openapi.json is excluded by !openapi/**
Files selected for processing: 2
  • console/v1/tests/consoleplugins.console.openshift.io/AAA_ungated.yaml
  • console/v1/types_console_plugin.go

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

Walkthrough

The ConsolePlugin CSP directive enum and documentation now include ObjectSrc. The documentation describes directive and value limits, how plugin directives combine with console defaults and other plugins, and an updated example. Validation tests cover an accepted ObjectSrc configuration and several rejected configurations.

Suggested reviewers: jhadvig

Priority: Normal

Merge Risk: Minimal · up to b70b9

The API schemas accept the new directive, and no issue remains that should prevent merging after normal checks.

Pre-merge checks: Passed 15
Check name Status Explanation
Title check Passed The title clearly and concisely identifies the primary change: adding ObjectSrc to ConsolePlugin CSP directives.
Description check Passed The description directly explains the ObjectSrc API change, validation behavior, documentation updates, tests, and rollout considerations.
Docstring Coverage Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1…
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 PASS. The pull request adds four schema-test names: "Should accept ObjectSrc CSP configuration", "Should reject MediaSrc while outside supported CSP scope", "Should reject duplicate ObjectSrc CSP dire…
Test Structure And Quality Passed The PR adds four table-driven API validation cases in YAML, not standalone Ginkgo It blocks. Each case checks one behavior and uses clear case names and expected results. The shared Ginkgo harness a…
Microshift Test Compatibility Passed PASS: The pull request adds ConsolePlugin schema validation cases in a YAML test fixture, not Ginkgo e2e tests. The changed tests validate ObjectSrc, MediaSrc rejection, duplicate directives, and quot…
Single Node Openshift (Sno) Test Compatibility Passed The pull request adds declarative ConsolePlugin API validation cases in console/v1/tests/consoleplugins.console.openshift.io/AAA_ungated.yaml. The changed tests use initial, expected, and `expec…
Topology-Aware Scheduling Compatibility Passed The pull request changes only ConsolePlugin API documentation, the ObjectSrc enum, generated CRD/OpenAPI/Swagger artifacts, and validation tests. It adds no deployment manifests, operator code, contro…
Ote Binary Stdout Contract Passed The pull request changes only ConsolePlugin API comments/schema artifacts and YAML validation fixtures. The added Go content is an enum constant and generated documentation/schema data; it adds no mai…
Ipv6 And Disconnected Network Test Compatibility Passed The pull request adds declarative ConsolePlugin validation cases in a YAML test fixture. It does not add Ginkgo e2e tests such as It, Describe, Context, or When. The example HTTPS host is only a valid…
No-Weak-Crypto Passed The pull request changes only ConsolePlugin CSP schema documentation, generated API artifacts, and validation tests. The added code introduces ObjectSrc and CSP validation rules. The changed lines c…
Container-Privileges Passed The pull request changes ConsolePlugin CSP API documentation, enum values, generated schemas, and validation tests. The diff adds no privileged, hostPID, hostNetwork, hostIPC, SYS_ADMIN, or …
No-Sensitive-Data-In-Logs Passed The pull request adds API schema comments, generated documentation, and validation fixtures only. It adds no logging calls or log output. The URLs use synthetic example domains, and no passwords, toke…
Full details: Docstring Coverage

Explanation

No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1 files. (1 skipped: 1 unsupported.)

Full details: Stable And Deterministic Test Names

Explanation

PASS. The pull request adds four schema-test names: "Should accept ObjectSrc CSP configuration", "Should reject MediaSrc while outside supported CSP scope", "Should reject duplicate ObjectSrc CSP directive entries", and "Should reject quoted ObjectSrc CSP values". The test generator passes these static YAML names directly to Ginkgo table entries. None contains generated suffixes, timestamps, node or namespace names, IPs, UUIDs, legacy metadata tokens, or other run-dependent values.

Full details: Test Structure And Quality

Explanation

The PR adds four table-driven API validation cases in YAML, not standalone Ginkgo It blocks. Each case checks one behavior and uses clear case names and expected results. The shared Ginkgo harness already installs and removes the CRD and deletes created resources in BeforeEach/AfterEach; its existing timeout covers CRD removal. No new cluster wait or assertion code was added. The informing recommendation does not apply to literal It() blocks in this change.

Full details: Microshift Test Compatibility

Explanation

PASS: The pull request adds ConsolePlugin schema validation cases in a YAML test fixture, not Ginkgo e2e tests. The changed tests validate ObjectSrc, MediaSrc rejection, duplicate directives, and quoted values. No It(), Describe(), Context(), or When() Ginkgo tests were added, so the MicroShift API compatibility check does not apply.

Full details: Single Node Openshift (Sno) Test Compatibility

Explanation

The pull request adds declarative ConsolePlugin API validation cases in console/v1/tests/consoleplugins.console.openshift.io/AAA_ungated.yaml. The changed tests use initial, expected, and expectedError YAML fields. The diff adds no Ginkgo It, Describe, Context, or When tests and makes no multi-node or HA assumptions. SNO compatibility protection is therefore not required.

Full details: Topology-Aware Scheduling Compatibility

Explanation

The pull request changes only ConsolePlugin API documentation, the ObjectSrc enum, generated CRD/OpenAPI/Swagger artifacts, and validation tests. It adds no deployment manifests, operator code, controllers, replicas, affinity, topology spread, node selectors, tolerations, or PDBs. Therefore it introduces no topology-aware scheduling constraint covered by this check.

Full details: Ote Binary Stdout Contract

Explanation

The pull request changes only ConsolePlugin API comments/schema artifacts and YAML validation fixtures. The added Go content is an enum constant and generated documentation/schema data; it adds no main(), suite setup, logging, or stdout write. The YAML cases are test data, not process-level code. No explicit OTE binary stdout violation is introduced.

Full details: Ipv6 And Disconnected Network Test Compatibility

Explanation

The pull request adds declarative ConsolePlugin validation cases in a YAML test fixture. It does not add Ginkgo e2e tests such as It, Describe, Context, or When. The example HTTPS host is only a validation input; the test does not connect to it. The IPv6 and disconnected-network check is therefore not applicable.

Full details: No-Weak-Crypto

Explanation

The pull request changes only ConsolePlugin CSP schema documentation, generated API artifacts, and validation tests. The added code introduces ObjectSrc and CSP validation rules. The changed lines contain no MD5, SHA1, DES, RC4, 3DES, Blowfish, ECB, custom cryptography, or secret/token comparisons.

Full details: Container-Privileges

Explanation

The pull request changes ConsolePlugin CSP API documentation, enum values, generated schemas, and validation tests. The diff adds no privileged, hostPID, hostNetwork, hostIPC, SYS_ADMIN, or allowPrivilegeEscalation settings, and it does not add a container running as root.

Full details: No-Sensitive-Data-In-Logs

Explanation

The pull request adds API schema comments, generated documentation, and validation fixtures only. It adds no logging calls or log output. The URLs use synthetic example domains, and no passwords, tokens, API keys, session IDs, PII, internal hostnames, or customer data are present.

Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

Warning

Some tools did not complete. Review the errors below.

🔧 golangci-lint (2.14.0)

golangci-lint (2.14.0) skipped for .golangci.yaml: configuration contains disallowed or uninspectable plugin or linter settings. Use supported built-in linters.

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

@openshift-ci

openshift-ci Bot commented Oct 26, 2026

Copy link
Copy Markdown
Contributor

Hello @shahsahil264! Some important instructions when contributing to openshift/api:
API design plays an important part in the user experience of OpenShift and as such API PRs are subject to a high level of scrutiny to ensure they follow our best practices. If you haven't already done so, please review the OpenShift API Conventions and ensure that your proposed changes are compliant. Following these conventions will help expedite the api review process for your PR.

@qodo-for-rh-openshift

Copy link
Copy Markdown

PR Summary by Qodo

Allow ObjectSrc in ConsolePlugin CSP directives

✨ Enhancement 🧪 Tests 📝 Documentation 🕐 20-40 Minutes


AI Description

• Allow plugins to declare ObjectSrc sources for object and embed content.
• Regenerate CRD and API documentation while retaining existing directive and value limits.
• Test ObjectSrc acceptance and rejection of unsupported, duplicate, and quoted declarations.
Diagram

graph TD
  P["Plugin configuration"] --> T["CSP API type"] --> C["CRD validation"] --> A["Plugin aggregation"] --> B["Bridge CSP"]
  T --> D["OpenAPI documentation"]
Loading
High-Level Assessment

Extending the existing directive enum and regenerating its schemas is the narrowest approach. A broader plugin-facing CSP API was considered but is unnecessary for ObjectSrc; keep other directives out of scope and coordinate deployment with Bridge support.

Files changed (7) +140 / -47

Enhancement (5) +72 / -44
types_console_plugin.goAdd ObjectSrc to the ConsolePlugin CSP API +24/-13

Add ObjectSrc to the ConsolePlugin CSP API

• Adds the ObjectSrc constant and validation enum entry. Clarifies aggregation, default sources, required values, and existing CSP limits in API comments without changing those limits.

console/v1/types_console_plugin.go

90_consoleplugins.crd.yamlExpose ObjectSrc in the ConsolePlugin CRD +20/-12

Expose ObjectSrc in the ConsolePlugin CRD

• Regenerates the CRD directive enum and field descriptions from the updated API definition.

console/v1/zz_generated.crd-manifests/90_consoleplugins.crd.yaml

AAA_ungated.yamlExpose ObjectSrc in the ungated CRD +20/-12

Expose ObjectSrc in the ungated CRD

• Regenerates the feature-gated CRD artifact with ObjectSrc in the allowed directive values and updated CSP descriptions.

console/v1/zz_generated.featuregated-crd-manifests/consoleplugins.console.openshift.io/AAA_ungated.yaml

zz_generated.openapi.goAdd ObjectSrc to generated Go OpenAPI schema +4/-4

Add ObjectSrc to generated Go OpenAPI schema

• Includes ObjectSrc in the directive enum and generated descriptions for ConsolePlugin CSP fields.

openapi/generated_openapi/zz_generated.openapi.go

openapi.jsonAdd ObjectSrc to published OpenAPI schema +4/-3

Add ObjectSrc to published OpenAPI schema

• Includes ObjectSrc in the published directive enum and refreshes the corresponding CSP descriptions.

openapi/openapi.json

Tests (1) +65 / -0
AAA_ungated.yamlCover ObjectSrc admission and invalid CSP declarations +65/-0

Cover ObjectSrc admission and invalid CSP declarations

• Adds an acceptance case for ObjectSrc and rejection cases for MediaSrc, duplicate ObjectSrc entries, and quoted ObjectSrc values.

console/v1/tests/consoleplugins.console.openshift.io/AAA_ungated.yaml

Documentation (1) +3 / -3
zz_generated.swagger_doc_generated.goRefresh generated ConsolePlugin Swagger descriptions +3/-3

Refresh generated ConsolePlugin Swagger descriptions

• Updates generated field documentation to describe ObjectSrc, aggregation behavior, and existing value requirements.

console/v1/zz_generated.swagger_doc_generated.go

@openshift-ci openshift-ci Bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Oct 26, 2026
@qodo-for-rh-openshift

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

Powered by Qodo

@openshift-ci
openshift-ci Bot requested review from JoelSpeed and jhadvig October 26, 2026 15:11
@openshift-ci

openshift-ci Bot commented Oct 26, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign joelspeed for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found 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 commented Oct 26, 2026

Copy link
Copy Markdown
Contributor

@shahsahil264: all tests passed!

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.

@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.

Adding this option reads as net-new functionality and should undergo the standard feature development process, which includes feature-gating this new value.

@shahsahil264

Copy link
Copy Markdown
Author

Thanks for the review! Following the team discussion, we changed the decision to keep object-src 'none' fixed in Console #17431. No API or operator change is needed, so I’m closing this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants