OTA-1956: install: Split multi-document YAML and add missing annotations#1423
OTA-1956: install: Split multi-document YAML and add missing annotations#1423jhadvig wants to merge 1 commit into
Conversation
…t YAML and add missing annotations""
openshift-ci-robot
added
the
jira/valid-reference
|
/payload-job periodic-ci-openshift-release-main-ci-5.0-e2e-aws-ovn-techpreview |
WalkthroughAdds the Kubernetes resources, image reference, service exposure, and OpenShift ConsolePlugin registration for the cluster-update console plugin. Updates manifest-rendering tests with the plugin image and excludes the image-reference file from annotation validation. ChangesCluster Update Console Plugin
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant OpenShiftConsole
participant ConsolePlugin
participant PluginService
participant PluginDeployment
OpenShiftConsole->>ConsolePlugin: load plugin configuration
ConsolePlugin->>PluginService: send requests to port 9001
PluginService->>PluginDeployment: select plugin pods
PluginDeployment-->>PluginService: serve HTTPS plugin content
🚥 Pre-merge checks | ✅ 14 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (14 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
@jhadvig: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/ea731e80-7f45-11f1-97f3-ddecb5eb0e9a-0 |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: jhadvig The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@install/0000_50_cluster-update-console-plugin_20_networkpolicy.yaml`:
- Around line 12-16: Update the NetworkPolicy containing podSelector {} and
policyTypes Ingress/Egress to add an ingress rule permitting traffic from the
openshift-console namespace. Preserve the existing default-deny behavior for all
other ingress sources and retain the current egress policy.
In `@install/0000_50_cluster-update-console-plugin_50_deployment.yaml`:
- Around line 38-41: Add CPU and memory limits to the container resources block
alongside the existing requests in the deployment manifest. Set bounded limit
values based on the plugin’s measured workload usage, ensuring both cpu and
memory are explicitly defined.
- Around line 16-20: Update the Deployment rollingUpdate strategy to use
maxUnavailable: 0 with an appropriate maxSurge to keep the single replica
available during rollout. Add HTTPS readiness and liveness probes targeting the
named https port, with suitable probe paths and settings. Update the associated
NetworkPolicy to allow the probe traffic.
- Around line 42-46: Update the container securityContext in the deployment
manifest to set readOnlyRootFilesystem to true. Preserve the existing read-only
config and certificate mounts, and add writable scratch mounts only if required
by the container image.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 26e0d70a-062c-4e66-9b45-379ce85a86c3
📒 Files selected for processing (9)
install/0000_50_cluster-update-console-plugin_10_namespace.yamlinstall/0000_50_cluster-update-console-plugin_15_serviceaccount.yamlinstall/0000_50_cluster-update-console-plugin_20_networkpolicy.yamlinstall/0000_50_cluster-update-console-plugin_30_configmap.yamlinstall/0000_50_cluster-update-console-plugin_50_deployment.yamlinstall/0000_50_cluster-update-console-plugin_60_service.yamlinstall/0000_50_cluster-update-console-plugin_90_consoleplugin.yamlinstall/image-referencespkg/payload/render_test.go
| spec: | ||
| podSelector: {} | ||
| policyTypes: | ||
| - Ingress | ||
| - Egress |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
set -euo pipefail
printf '\n## File list\n'
git ls-files install/0000_50_cluster-update-console-plugin_20_networkpolicy.yaml install/0000_50_cluster-update-console-plugin_90_consoleplugin.yaml
printf '\n## NetworkPolicy file\n'
cat -n install/0000_50_cluster-update-console-plugin_20_networkpolicy.yaml
printf '\n## ConsolePlugin file\n'
cat -n install/0000_50_cluster-update-console-plugin_90_consoleplugin.yaml
printf '\n## Related NetworkPolicies under install\n'
fd -a '.*networkpolicy.*\.ya?ml$' install
printf '\n## Search for network policy and console plugin references\n'
rg -n --hidden --glob 'install/**' -e 'kind: NetworkPolicy|kind: ConsolePlugin|namespaceSelector|podSelector|policyTypes|ingress:|egress:' installRepository: openshift/cluster-version-operator
Length of output: 2985
🏁 Script executed:
set -euo pipefail
echo hiRepository: openshift/cluster-version-operator
Length of output: 175
🏁 Script executed:
set -euo pipefail
echo '--- networkpolicy file ---'
cat -n install/0000_50_cluster-update-console-plugin_20_networkpolicy.yaml
echo '--- consoleplugin file ---'
cat -n install/0000_50_cluster-update-console-plugin_90_consoleplugin.yaml
echo '--- install networkpolicy files ---'
fd -a '.*networkpolicy.*\.ya?ml$' install
echo '--- relevant references ---'
rg -n --hidden --glob 'install/**' -e 'kind: NetworkPolicy|kind: ConsolePlugin|namespaceSelector|podSelector|policyTypes|ingress:|egress:' installRepository: openshift/cluster-version-operator
Length of output: 2805
🏁 Script executed:
pwdRepository: openshift/cluster-version-operator
Length of output: 191
🌐 Web query:
OpenShift ConsolePlugin backend service network policy source ingress router namespace
💡 Result:
To configure network policies for an OpenShift ConsolePlugin, you must account for the traffic flow initiated by the OpenShift web console backend. The console backend proxies requests to your plugin service and, if configured, to additional in-cluster services via the ConsolePlugin service proxy [1][2][3]. Key points for network policy configuration: 1. Console Backend to Plugin Service: The OpenShift console backend initiates connections to your plugin's service (specified in spec.backend.service) to fetch assets and extension manifests [4][5][6]. Your network policy in the plugin's namespace must allow ingress traffic from the namespace where the OpenShift console is running (typically the openshift-console namespace) to the plugin service's port [4][7]. 2. Service Proxy Traffic: If your plugin uses the ConsolePlugin service proxy feature (spec.proxy) to communicate with other in-cluster services, the console backend acts as the proxy [1][2][3]. Requests to these services originate from the console backend pod, not directly from your plugin's pod [1][2]. Consequently, network policies for those target services must also allow ingress from the openshift-console namespace [1][2]. 3. Router/Ingress Considerations: ConsolePlugin backend services do not require an OpenShift Route or Ingress to be exposed to the outside world [4][5]. The console backend communicates with them over the internal cluster network using the service name and namespace [4][6]. Because the console backend uses the service CA bundle for secure communication, your plugin service must support HTTPS and be configured with a serving certificate (often via the service-ca operator) [4][7][3]. In summary, you should ensure that your network policies permit ingress to your plugin service specifically from the OpenShift console namespace [4][1]. You do not need to expose these services via a router/ingress [4]. If your plugin proxies requests to other services, verify that those target services allow ingress from the console namespace as well [1][2].
Citations:
- 1: https://docs.okd.io/4.22/web_console/dynamic-plugin/deploy-plugin-cluster.html
- 2: https://docs.okd.io/4.19/web_console/dynamic-plugin/deploy-plugin-cluster.html
- 3: https://github.com/openshift/console/blob/master/dynamic-demo-plugin/README.md
- 4: https://docs.okd.io/latest/rest_api/console_apis/consoleplugin-console-openshift-io-v1.html
- 5: https://github.com/openshift/api/blob/master/console/v1/types_console_plugin.go
- 6: https://docs.redhat.com/en/documentation/openshift_container_platform/4.15/html/console_apis/consoleplugin-console-openshift-io-v1
- 7: https://github.com/openshift/api/blob/e83d48817f01601c3ff7c39f53fc28342965bc55/console/v1alpha1/0000_10_consoleplugin.crd.yaml
Allow ingress from openshift-console. This namespace-wide default-deny policy has no ingress rule, so the ConsolePlugin backend service in openshift-cluster-update-console-plugin will be unreachable from the console unless another policy adds that exception.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@install/0000_50_cluster-update-console-plugin_20_networkpolicy.yaml` around
lines 12 - 16, Update the NetworkPolicy containing podSelector {} and
policyTypes Ingress/Egress to add an ingress rule permitting traffic from the
openshift-console namespace. Preserve the existing default-deny behavior for all
other ingress sources and retain the current egress policy.
| strategy: | ||
| rollingUpdate: | ||
| maxSurge: 25% | ||
| maxUnavailable: 1 | ||
| type: RollingUpdate |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Prevent rollout downtime and add health probes.
With replicas omitted, this Deployment runs one pod, while maxUnavailable: 1 permits zero available pods during a rollout. It also has no readiness or liveness probes, so traffic can reach a merely-running container and a hung Nginx process will not be restarted.
Set maxUnavailable: 0 with an appropriate surge, and add HTTPS readiness/liveness probes on the named https port. Ensure the NetworkPolicy also permits the required probe traffic.
As per path instructions, Kubernetes/OpenShift manifests must define liveness and readiness probes.
Also applies to: 31-38
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@install/0000_50_cluster-update-console-plugin_50_deployment.yaml` around
lines 16 - 20, Update the Deployment rollingUpdate strategy to use
maxUnavailable: 0 with an appropriate maxSurge to keep the single replica
available during rollout. Add HTTPS readiness and liveness probes targeting the
named https port, with suitable probe paths and settings. Update the associated
NetworkPolicy to allow the probe traffic.
Source: Path instructions
| resources: | ||
| requests: | ||
| cpu: 20m | ||
| memory: 50Mi |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Add CPU and memory limits.
Only resource requests are defined. Without limits, a faulty or unexpectedly busy plugin can consume unbounded node resources. Add bounded CPU and memory limits based on measured workload usage.
As per path instructions, every Kubernetes container must define CPU and memory resource limits.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@install/0000_50_cluster-update-console-plugin_50_deployment.yaml` around
lines 38 - 41, Add CPU and memory limits to the container resources block
alongside the existing requests in the deployment manifest. Set bounded limit
values based on the plugin’s measured workload usage, ensuring both cpu and
memory are explicitly defined.
Source: Path instructions
| securityContext: | ||
| allowPrivilegeEscalation: false | ||
| capabilities: | ||
| drop: | ||
| - ALL |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the relevant deployment file with line numbers
sed -n '1,220p' install/0000_50_cluster-update-console-plugin_50_deployment.yaml | cat -n
# Find any existing readOnlyRootFilesystem settings in the repo
rg -n "readOnlyRootFilesystem|allowPrivilegeEscalation|securityContext|volumeMounts|emptyDir|configMap|secret" install/0000_50_cluster-update-console-plugin_50_deployment.yaml install -g '!**/vendor/**' -g '!**/node_modules/**' || true
# Locate the image/build definition for this workload
rg -n "cluster-update-console-plugin|console-plugin|static content|nginx|httpd|serve" . -g '!**/vendor/**' -g '!**/node_modules/**' || trueRepository: openshift/cluster-version-operator
Length of output: 50390
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show only the relevant section if the file is large
wc -l install/0000_50_cluster-update-console-plugin_50_deployment.yaml
sed -n '1,140p' install/0000_50_cluster-update-console-plugin_50_deployment.yaml | cat -nRepository: openshift/cluster-version-operator
Length of output: 3276
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect any image/package references that might imply writable runtime paths
rg -n "cluster-update-console-plugin|console plugin|nginx|httpd|dist/|/tmp|/var/cache|/var/run|/usr/share" . -g '!**/vendor/**' -g '!**/node_modules/**' || trueRepository: openshift/cluster-version-operator
Length of output: 6593
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map nearby manifests and templates for this component
git ls-files install | rg 'cluster-update-console-plugin|console-plugin' || trueRepository: openshift/cluster-version-operator
Length of output: 632
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file=install/0000_50_cluster-update-console-plugin_50_deployment.yaml
wc -l "$file"
sed -n '1,180p' "$file" | cat -nRepository: openshift/cluster-version-operator
Length of output: 3276
Enable readOnlyRootFilesystem for this container. The existing config and cert mounts are already read-only; if the image needs scratch space, add only the required writable mounts.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@install/0000_50_cluster-update-console-plugin_50_deployment.yaml` around
lines 42 - 46, Update the container securityContext in the deployment manifest
to set readOnlyRootFilesystem to true. Preserve the existing read-only config
and certificate mounts, and add writable scratch mounts only if required by the
container image.
Source: Path instructions
|
/test ci/prow/e2e-agnostic-operator ci/prow/e2e-agnostic-ovn-techpreview-serial-1of3 ci/prow/e2e-agnostic-ovn-techpreview-serial-3of3 ci/prow/e2e-agnostic-ovn-upgrade-out-of-change |
|
/test e2e-agnostic-operator e2e-agnostic-ovn-techpreview-serial-1of3 e2e-agnostic-ovn-techpreview-serial-3of3 e2e-agnostic-ovn-upgrade-out-of-change |
|
/retest |
|
/payload-job periodic-ci-openshift-release-main-ci-5.0-e2e-aws-ovn-techpreview |
|
@jhadvig: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/771cf5b0-7f94-11f1-8e4e-1916fa640362-0 |
2 participants
Reverts #1420
Original PR #1398
ccing @wking
Summary by CodeRabbit
New Features
Tests