This repository was archived by the owner on Sep 9, 2026. It is now read-only.
Add Helm-based VMaaS CI deployment path - #296
Merged
omer-vishlitzky merged 15 commits intoJun 20, 2026
Merged
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: omer-vishlitzky The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds AAP ChangesHelm Chart: Values, RBAC, Instance Groups, and Hooks
Installer Scripts: Namespace Resolution, Fulfillment Patching, and Snapshot Refresh
Values Layout Migration, CI, Tooling, and Environment Overrides
Sequence Diagram(s)sequenceDiagram
rect rgba(173, 216, 230, 0.5)
Note over main,post_flight: scripts/refresh-after-snapshot.py
participant main as main()
participant P1 as Phase 1 (parallel)
participant P2 as Phase 2 (parallel)
participant P3 as Phase 3 (sequential)
participant P4 as post_flight
end
main->>P1: patch_stale_routes, refresh_cdi, refresh_metallb, wait_keycloak_cert, pre_fix_cert_sans
P1-->>main: done
main->>P2: keycloak_sync, create_secrets, ensure_ca_bundle, wait_tls_certs, scale_authorino
P2-->>main: done
main->>P3: upgrade_fulfillment_db, upgrade_osac (adopt + helm upgrade), wait_fulfillment
P3->>P3: scale_aap_operator, wait_aap_ready (HTTP 200 + rollout), fix_assisted_service
P3-->>main: done
main->>P4: export env, prepare-aap.sh, prepare-fulfillment-service.sh, prepare-tenant.sh
P4-->>main: done
main->>main: check_all_pods, print timing
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
Poem
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (2 errors, 2 warnings)
✅ Passed checks (7 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 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 `@charts/osac/templates/aap-service-accounts.yaml`:
- Around line 19-32: Rather than continue with the temporary workaround that
grants cluster-admin to the osac-sa ServiceAccount (which violates
least-privilege RBAC), coordinate with the osac-aap upstream maintainers to
resolve the root cause: the naming mismatch between the osac-aap subchart's
ServiceAccount prefix (osac-aap-) and the hardcoded ServiceAccount references
(osac-sa and template-publisher) in the AAP config-as-code playbooks. Work with
the osac-aap team to either remove the prefix from their subchart or update the
playbooks to use the prefixed names. Once the upstream naming issue is resolved,
remove the entire aap-service-accounts.yaml file and its ClusterRoleBinding that
currently grants overprivileged cluster-admin access.
In `@charts/osac/templates/cluster-fulfillment-ig.yaml`:
- Line 5: The $baseDomain variable on line 5 computes a value by trimming
"apps." from the ingress domain, but this diverges from the snapshot refresh
flow in scripts/refresh-after-snapshot-helm.py which sets
HOSTED_CLUSTER_BASE_DOMAIN with a "hosted." prefix prepended to the trimmed
domain. Update the $baseDomain assignment on line 5 to include the same
"hosted." prefix that is used in the refresh flow, ensuring both code paths
produce consistent values for the same domain format.
In `@charts/osac/templates/hooks/patch-authconfig.yaml`:
- Around line 46-51: The while True loop that polls the AuthConfig for
"fulfillment-service" with subprocess.run lacks a timeout mechanism, causing
indefinite hangs if the resource never exists. Add a timeout variable
initialized before the loop that tracks elapsed time, increment it on each
iteration after time.sleep(5), and break out of the loop if the timeout is
exceeded, raising an appropriate exception to fail the job gracefully.
Alternatively or additionally, add activeDeadlineSeconds to the Job spec in the
YAML to enforce a hard deadline at the Kubernetes level.
In `@charts/osac/values.schema.json`:
- Line 97: The schema in values.schema.json is missing definitions for new
configuration keys that have been introduced in values.yaml. Add schema property
definitions for the missing keys under the aap.configAsCode object (specifically
for eeImage, projectGitUri, and projectGitBranch) and for the instanceGroups
property to match the actual structure now present in values.yaml. Update the
required array as needed to reflect which of these new properties are mandatory,
ensuring the schema contract fully represents the current configuration
structure to prevent silent misconfiguration via typos.
In `@scripts/prepare-aap.sh`:
- Around line 36-48: Quote the `${INSTALLER_NAMESPACE}` variable on lines 38 and
46 to prevent word splitting and globbing issues. Wrap the variable in double
quotes in both the `oc create secret generic` command and the `oc set env`
command. Additionally, review the `grep osac-operator` pattern on line 43 in the
OPERATOR_DEPLOY assignment: if multiple deployments could match this pattern,
consider using a more specific grep pattern or pipe the result to `head -1` to
ensure only one deployment is selected.
In `@scripts/refresh-after-snapshot-helm.py`:
- Line 23: Change the import statement for Callable from the typing module to
collections.abc module. Replace the line `from typing import Callable` with an
import from collections.abc to align with modern Python standards and Ruff's
UP035 rule recommendation.
- Around line 326-346: The next() call in the create_secrets function that
searches for a client with serviceAccountsEnabled enabled lacks a default value,
which means if no such client exists it will raise a StopIteration exception
without providing helpful context. Fix this by either providing a default value
to the next() call (with None or a descriptive error message) or by wrapping the
next() call in a try-except block to catch StopIteration and raise a
RuntimeError with a clear message explaining that no service account enabled
client was found in the realm configuration.
- Around line 276-314: The subprocess.run calls for client and user upsert
operations (the PUT/POST curl commands that update or create clients and users)
are not checking the HTTP response codes, so failures are silently ignored. For
both the client upsert subprocess.run call (around line 290-296) and the user
upsert subprocess.run call (around line 308-314), add the "-o /dev/null -w
%{http_code}" flags to the curl command to capture the HTTP status code, then
check the captured response to verify the status code is in the 2xx range, and
either log or exit on non-2xx responses to prevent leaving the realm in an
inconsistent state.
🪄 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: osac-project/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: c37c146d-6006-45f7-ab9e-77c158ed4291
⛔ Files ignored due to path filters (1)
charts/osac/Chart.lockis excluded by!**/*.lock
📒 Files selected for processing (20)
charts/osac/Chart.yamlcharts/osac/charts/fulfillment-service-0.0.0.tgzcharts/osac/charts/osac-aap-0.0.0.tgzcharts/osac/charts/osac-operator-0.0.0.tgzcharts/osac/charts/osac-operator-crds-0.0.0.tgzcharts/osac/templates/aap-service-accounts.yamlcharts/osac/templates/cluster-fulfillment-ig.yamlcharts/osac/templates/hooks/patch-authconfig.yamlcharts/osac/templates/hooks/pre-install-validate.yamlcharts/osac/templates/hub-access.yamlcharts/osac/templates/instance-groups.yamlcharts/osac/templates/operator-rbac-supplement.yamlcharts/osac/values.schema.jsoncharts/osac/values.yamlprerequisites/keycloak/service/files/realm.jsonscripts/prepare-aap.shscripts/prepare-fulfillment-service.shscripts/prepare-tenant.shscripts/refresh-after-snapshot-helm.pyvalues/vmaas-ci/values.yaml
💤 Files with no reviewable changes (1)
- charts/osac/Chart.yaml
omer-vishlitzky
force-pushed
the
vmaas-helm-ci
branch
4 times, most recently
from
June 18, 2026 17:18
5543dc5 to
4202a46
Compare
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 `@charts/osac/templates/hooks/patch-authconfig.yaml`:
- Around line 63-66: The rego string replacement in this code block is fragile
and does not validate that the expected pattern exists before replacing it. Add
a check before the replace() call on the rego variable to verify that the exact
pattern `"system:serviceaccount:{NS}:admin",` exists in the rego string. If the
pattern is not found (which could happen due to upstream format changes,
different quoting/whitespace, or removal of the admin service account), raise an
explicit error and fail the script rather than silently continuing with the
unchanged rego variable. This ensures the script fails fast with a clear error
message when the expected injection point is not found, preventing silent
failures and malformed Rego output.
In `@charts/osac/templates/instance-groups.yaml`:
- Around line 12-14: The instance-group config rendering in the range loop over
.Values.instanceGroups.cluster.config is emitting keys with empty values, which
can unintentionally override application defaults with blank strings. Add a
conditional check within the range loop (at line 12) to skip rendering entries
when the value is empty, similar to the existing network-fulfillment-ig pattern.
This same fix should also be applied to the second occurrence mentioned at lines
36-38, ensuring both config blocks only emit keys when their corresponding
values are non-empty.
In `@scripts/refresh-after-snapshot-helm.py`:
- Around line 262-271: Add a clear warning to the script's docstring at the top
of the refresh-after-snapshot-helm.py file documenting that this script is
intended for CI/test environments only and must never be run against production
Keycloak instances, since it uses hardcoded admin credentials (password=admin)
in the curl command that authenticates with Keycloak around line 265. This
warning will help prevent accidental execution against production systems.
- Around line 236-240: In the wait_keycloak_cert function, the string literal
f"certificate/keycloak-tls" on line 238 is marked as an f-string with the f
prefix but contains no variable placeholders. Remove the f prefix from this
string literal so it becomes a regular string "certificate/keycloak-tls"
instead.
🪄 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: osac-project/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 9a6f2baf-b2f9-4e53-ad3d-19828d5a3340
📒 Files selected for processing (14)
charts/osac/templates/aap-service-accounts.yamlcharts/osac/templates/cluster-fulfillment-ig.yamlcharts/osac/templates/hooks/patch-authconfig.yamlcharts/osac/templates/hooks/pre-install-validate.yamlcharts/osac/templates/hub-access.yamlcharts/osac/templates/instance-groups.yamlcharts/osac/templates/operator-rbac-supplement.yamlcharts/osac/values.schema.jsoncharts/osac/values.yamlscripts/prepare-aap.shscripts/prepare-fulfillment-service.shscripts/prepare-tenant.shscripts/refresh-after-snapshot-helm.pyvalues/vmaas-ci/values.yaml
omer-vishlitzky
force-pushed
the
vmaas-helm-ci
branch
from
June 18, 2026 19:58
9f42c5b to
87bcff6
Compare
Contributor
Author
|
/hold |
eliorerz
suggested changes
Jun 19, 2026
| {{- $ingress := lookup "config.openshift.io/v1" "Ingress" "" "cluster" }} | ||
| {{- $ingressDomain := "" }} | ||
| {{- if $ingress }}{{- $ingressDomain = $ingress.spec.domain | default "" }}{{- end }} | ||
| {{- $baseDomain := $ingressDomain | trimPrefix "apps." }} |
Contributor
There was a problem hiding this comment.
You are calculating base domain different here and in scripts/refresh-after-snapshot.py. Is there a reason for that?
| @@ -1,4 +1,8 @@ | |||
| {{- if .Values.clusterFulfillment.enabled }} | |||
| {{- $ingress := lookup "config.openshift.io/v1" "Ingress" "" "cluster" }} | |||
Contributor
There was a problem hiding this comment.
lookup will return empty during helm template / CI dry-runs, so in that case $baseDomain will be an empty string (and HOSTED_CLUSTER_BASE_DOMAIN silently defaults to "").
Comment on lines
+8
to
+13
| labels: | ||
| {{- include "osac.labels" . | nindent 4 }} | ||
| osac.openshift.io/project: osac-aap | ||
| data: | ||
| {{- range $key, $value := .Values.instanceGroups.cluster.config }} | ||
| {{ $key }}: {{ $value | quote }} |
Contributor
There was a problem hiding this comment.
This creates cluster-fulfillment-ig ConfigMap, but cluster-fulfillment-ig.yaml also creates one with the same name when clusterFulfillment.enabled is true. if both flags are on, these collide. Am I missing something?
| "--overwrite", check=False) | ||
|
|
||
|
|
||
| def upgrade_osac(config: RefreshConfig) -> None: |
Contributor
There was a problem hiding this comment.
I've just fixed this in PR #283 lol. Helm dependency build blows up when Chart.lock is out of sync. should be helm dependency update here too.
| oc set env "${OPERATOR_DEPLOY}" \ | ||
| -n ${INSTALLER_NAMESPACE} \ | ||
| OSAC_AAP_URL="${AAP_URL}/api/controller" \ | ||
| OSAC_AAP_TOKEN="${AAP_TOKEN}" |
Contributor
There was a problem hiding this comment.
I think that now the token might be visible in cleartext in the deployment spec.
omer-vishlitzky
force-pushed
the
vmaas-helm-ci
branch
from
June 19, 2026 21:58
87bcff6 to
15fdcc2
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
charts/osac/templates/hooks/pre-install-validate.yaml (2)
22-26:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd securityContext and resource limits to the validation container.
Per Kubernetes/OpenShift hardening guidelines, this container spec is missing:
securityContextwithrunAsNonRoot: true,readOnlyRootFilesystem: true,allowPrivilegeEscalation: false, andcapabilities.drop: ["ALL"]resources.requestsandresources.limitsfor CPU and memoryThese constraints are required for all container specs in the chart.
🔒 Proposed fix: Add security context and resource limits
- name: validate image: {{ .Values.validation.image }} + securityContext: + runAsNonRoot: true + readOnlyRootFilesystem: true + allowPrivilegeEscalation: false + capabilities: + drop: + - ALL + resources: + requests: + cpu: 100m + memory: 128Mi + limits: + cpu: 500m + memory: 256Mi command: - /bin/sh🤖 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 `@charts/osac/templates/hooks/pre-install-validate.yaml` around lines 22 - 26, The validate container spec in the pre-install-validate.yaml file is missing critical security hardening configuration. Add a securityContext section to the validate container with runAsNonRoot set to true, readOnlyRootFilesystem set to true, allowPrivilegeEscalation set to false, and capabilities.drop containing ALL. Additionally, add a resources section with both requests and limits for CPU and memory to ensure proper resource governance. These fields should be added as sibling properties to the existing command property in the validate container spec.Source: Coding guidelines
22-22:⚠️ Potential issue | 🔴 CriticalAdd securityContext and resource limits to all containers across the chart.
All containers in the chart (hooks, main workloads) are missing required security hardening. Implement:
securityContext:runAsNonRoot: true,readOnlyRootFilesystem: true,allowPrivilegeEscalation: false, and drop ALL capabilitiesresources.limitsandresources.requestsforcpuandmemoryon every containerStart with the hook Jobs (
pre-install-validate,patch-authconfig,db-migrate) and apply consistently to all other templates.🤖 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 `@charts/osac/templates/hooks/pre-install-validate.yaml` at line 22, Add hardening configurations to the validate container in the pre-install-validate.yaml hook file by including a securityContext block with runAsNonRoot set to true, readOnlyRootFilesystem set to true, allowPrivilegeEscalation set to false, and capabilities drop set to ALL, along with a resources block specifying both limits and requests for cpu and memory. After applying these changes to the validate container in pre-install-validate, replicate the exact same securityContext and resources configuration to all containers in the other hook files (patch-authconfig and db-migrate) and then extend the pattern to all main workload templates throughout the chart to ensure consistent security hardening across all containers.Source: Coding guidelines
♻️ Duplicate comments (4)
scripts/refresh-after-snapshot.py (1)
437-441:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
next()without default raises opaqueStopIterationon missing client.If no client in
realm.jsonhasserviceAccountsEnabled, this raisesStopIterationwith no context. The subsequentRuntimeErroron line 441 is never reached because the failure happens at line 437.Proposed fix
- fc_client = next(c for c in realm["clients"] if c.get("serviceAccountsEnabled")) + fc_client = next( + (c for c in realm["clients"] if c.get("serviceAccountsEnabled")), + None, + ) + if fc_client is None: + raise RuntimeError("No client with serviceAccountsEnabled in realm.json") fc_id: str = fc_client["clientId"]🤖 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 `@scripts/refresh-after-snapshot.py` around lines 437 - 441, The next() function call at line 437 lacks a default value, causing an opaque StopIteration exception when no client with serviceAccountsEnabled is found in realm["clients"], preventing the descriptive RuntimeError from being raised. Fix this by either providing a default value of None to next() and then explicitly checking if fc_client is None before proceeding, or by catching the StopIteration exception and raising a descriptive RuntimeError that explains no client with serviceAccountsEnabled was found in realm.json.charts/osac/values.schema.json (1)
480-493:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
instanceGroupsschema is too loose for template expectations.Templates consume
.Values.instanceGroups.cluster.configand.Values.instanceGroups.network.config, but this schema only constrainscluster/networkas generic objects. Please type the nestedconfigobjects explicitly so invalid shapes are rejected before render time.Suggested schema tightening
"instanceGroups": { "type": "object", "properties": { "enabled": { "type": "boolean" }, "cluster": { - "type": "object" + "type": "object", + "properties": { + "config": { "type": "object" } + } }, "network": { - "type": "object" + "type": "object", + "properties": { + "config": { "type": "object" } + } } } }🤖 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 `@charts/osac/values.schema.json` around lines 480 - 493, The `cluster` and `network` properties within `instanceGroups` are defined as generic objects without specifying their expected structure. Since templates consume `.Values.instanceGroups.cluster.config` and `.Values.instanceGroups.network.config`, you need to add explicit property definitions to both the `cluster` and `network` object schemas. Define a `config` property within each (as an object type) so that only valid shapes matching the template expectations are allowed, preventing invalid configurations from passing validation.charts/osac/templates/instance-groups.yaml (1)
13-15:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winSkip empty instance-group entries when rendering ConfigMaps.
These loops currently emit keys even when value is empty, which can override defaults with blank strings in downstream consumers.
Suggested guard
data: {{- range $key, $value := .Values.instanceGroups.cluster.config }} + {{- if $value }} {{ $key }}: {{ $value | quote }} + {{- end }} {{- end }} @@ data: {{- range $key, $value := .Values.instanceGroups.network.config }} + {{- if $value }} {{ $key }}: {{ $value | quote }} + {{- end }} {{- end }}Also applies to: 38-40
🤖 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 `@charts/osac/templates/instance-groups.yaml` around lines 13 - 15, The template loops iterating over `.Values.instanceGroups.cluster.config` are rendering all key-value pairs without checking if values are empty, which causes empty strings to override defaults in downstream consumers. Add a conditional guard around the line that outputs `{{ $key }}: {{ $value | quote }}` to skip rendering when the value is empty or not set. Apply this same fix to both occurrences of the range loop (the one shown in the diff and the similar one referenced at lines 38-40).charts/osac/templates/hooks/patch-authconfig.yaml (1)
65-71:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRego patching can report success without inserting principals.
Line 68 relies on exact string formatting, and
changedis set on Line 71 even ifreplace()makes no substitution. If upstream Rego format drifts, this can print success and apply unchanged policy.Suggested fail-fast validation
changed = False + injection_point = f'"system:serviceaccount:{NS}:admin",' + missing = [] for sa in EXTRA_SAS: full = f"system:serviceaccount:{NS}:{sa}" if full not in rego: - rego = rego.replace( - f'"system:serviceaccount:{NS}:admin",', - f'"system:serviceaccount:{NS}:admin",\n "{full}",') - changed = True - print(f"Adding {full}") + missing.append(full) + + if missing and injection_point not in rego: + print("ERROR: expected admin injection point not found in Rego", file=sys.stderr) + sys.exit(1) + + for full in missing: + new_rego = rego.replace( + injection_point, + f'{injection_point}\n "{full}",', + 1, + ) + if new_rego == rego: + print(f"ERROR: failed to insert {full}", file=sys.stderr) + sys.exit(1) + rego = new_rego + changed = True + print(f"Adding {full}")Also applies to: 74-80
🤖 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 `@charts/osac/templates/hooks/patch-authconfig.yaml` around lines 65 - 71, The code in the EXTRA_SAS loop sets the changed flag to True based only on whether full is not in rego, but does not verify that the rego.replace() call actually found and replaced the target string. If the exact string format (containing "system:serviceaccount:{NS}:admin",) does not exist in the upstream Rego due to formatting changes, the replace() method will return the original string unchanged while changed is still set to True, causing the code to incorrectly report success without actually inserting the new principal. Modify the logic to verify that the rego.replace() call actually made a substitution before setting changed to True, such as by comparing the resulting rego string to the original to confirm the replacement occurred.
🤖 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 `@charts/osac/Chart.yaml`:
- Around line 28-33: The Chart.yaml file references `bmf.enabled` as a condition
in the bare-metal-fulfillment-operator dependency (appearing on lines 28 and
33), but the values schema file at charts/osac/values.schema.json does not
define the `bmf` object or its `enabled` property. Add a schema definition for
the `bmf` object to values.schema.json that includes an `enabled` property with
type boolean to properly validate this configuration option and prevent
mis-typed values from affecting dependency resolution.
In `@charts/osac/templates/hooks/patch-authconfig.yaml`:
- Around line 22-23: The Kubernetes Job for patching AuthConfig lacks hard
timeout controls, allowing hook execution to stall indefinitely. Add an
activeDeadlineSeconds field to the Job spec at the same level as backoffLimit to
enforce a maximum wall-clock timeout for the entire hook execution.
Additionally, for any subprocess.run calls within the script executed by this
Job (referenced in the locations at lines 49-50, 58-60, and 79-80), add a
timeout parameter to each call to prevent individual subprocess calls from
blocking indefinitely. This ensures both job-level and subprocess-level
execution boundaries are enforced during Helm post-install and post-upgrade
operations.
---
Outside diff comments:
In `@charts/osac/templates/hooks/pre-install-validate.yaml`:
- Around line 22-26: The validate container spec in the
pre-install-validate.yaml file is missing critical security hardening
configuration. Add a securityContext section to the validate container with
runAsNonRoot set to true, readOnlyRootFilesystem set to true,
allowPrivilegeEscalation set to false, and capabilities.drop containing ALL.
Additionally, add a resources section with both requests and limits for CPU and
memory to ensure proper resource governance. These fields should be added as
sibling properties to the existing command property in the validate container
spec.
- Line 22: Add hardening configurations to the validate container in the
pre-install-validate.yaml hook file by including a securityContext block with
runAsNonRoot set to true, readOnlyRootFilesystem set to true,
allowPrivilegeEscalation set to false, and capabilities drop set to ALL, along
with a resources block specifying both limits and requests for cpu and memory.
After applying these changes to the validate container in pre-install-validate,
replicate the exact same securityContext and resources configuration to all
containers in the other hook files (patch-authconfig and db-migrate) and then
extend the pattern to all main workload templates throughout the chart to ensure
consistent security hardening across all containers.
---
Duplicate comments:
In `@charts/osac/templates/hooks/patch-authconfig.yaml`:
- Around line 65-71: The code in the EXTRA_SAS loop sets the changed flag to
True based only on whether full is not in rego, but does not verify that the
rego.replace() call actually found and replaced the target string. If the exact
string format (containing "system:serviceaccount:{NS}:admin",) does not exist in
the upstream Rego due to formatting changes, the replace() method will return
the original string unchanged while changed is still set to True, causing the
code to incorrectly report success without actually inserting the new principal.
Modify the logic to verify that the rego.replace() call actually made a
substitution before setting changed to True, such as by comparing the resulting
rego string to the original to confirm the replacement occurred.
In `@charts/osac/templates/instance-groups.yaml`:
- Around line 13-15: The template loops iterating over
`.Values.instanceGroups.cluster.config` are rendering all key-value pairs
without checking if values are empty, which causes empty strings to override
defaults in downstream consumers. Add a conditional guard around the line that
outputs `{{ $key }}: {{ $value | quote }}` to skip rendering when the value is
empty or not set. Apply this same fix to both occurrences of the range loop (the
one shown in the diff and the similar one referenced at lines 38-40).
In `@charts/osac/values.schema.json`:
- Around line 480-493: The `cluster` and `network` properties within
`instanceGroups` are defined as generic objects without specifying their
expected structure. Since templates consume
`.Values.instanceGroups.cluster.config` and
`.Values.instanceGroups.network.config`, you need to add explicit property
definitions to both the `cluster` and `network` object schemas. Define a
`config` property within each (as an object type) so that only valid shapes
matching the template expectations are allowed, preventing invalid
configurations from passing validation.
In `@scripts/refresh-after-snapshot.py`:
- Around line 437-441: The next() function call at line 437 lacks a default
value, causing an opaque StopIteration exception when no client with
serviceAccountsEnabled is found in realm["clients"], preventing the descriptive
RuntimeError from being raised. Fix this by either providing a default value of
None to next() and then explicitly checking if fc_client is None before
proceeding, or by catching the StopIteration exception and raising a descriptive
RuntimeError that explains no client with serviceAccountsEnabled was found in
realm.json.
🪄 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: osac-project/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: a3258e14-7cea-4224-9575-eceb88ee5507
📒 Files selected for processing (33)
.github/workflows/helm-integration.yaml.github/workflows/helm-lint.yaml.gitignoreREADME.mdbase/bare-metal-fulfillment-operatorbase/kustomization.yamlbase/osac-aapbase/osac-fulfillment-servicebase/osac-operatorcharts/osac/Chart.yamlcharts/osac/templates/cluster-fulfillment-ig.yamlcharts/osac/templates/hooks/patch-authconfig.yamlcharts/osac/templates/hooks/pre-install-validate.yamlcharts/osac/templates/hub-access.yamlcharts/osac/templates/instance-groups.yamlcharts/osac/templates/operator-rbac-supplement.yamlcharts/osac/values.schema.jsoncharts/osac/values.yamldocs/helm-deployment-guide.mddocs/network-backend.mdoverlays/caas-ci/kustomization.yamloverlays/development/kustomization.yamloverlays/osac-integration/kustomization.yamloverlays/vmaas-ci/kustomization.yamlscripts/prepare-aap.shscripts/prepare-fulfillment-service.shscripts/prepare-tenant.shscripts/refresh-after-snapshot.pyscripts/setup.shscripts/sync-image-tags.shvalues/caas-ci/values.yamlvalues/development/values.yamlvalues/vmaas-ci/values.yaml
omer-vishlitzky
force-pushed
the
vmaas-helm-ci
branch
2 times, most recently
from
June 19, 2026 23:05
6aa68a1 to
b5dc045
Compare
Contributor
Author
|
/retest |
- Add patch-authconfig post-install hook to inject emergency SAs - Add instance-groups and operator-rbac-supplement templates - Add condition: bmf.enabled to BMF chart dependencies so environments that don't need bare-metal fulfillment can disable it entirely - Add bmf.enabled: true default in values.yaml - Update values schema with new fields Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Move values files from values/<env>.yaml to values/<env>/values.yaml so each environment directory can hold additional files (e.g. license.zip). VMaaS CI values set bmf.enabled: false since bare-metal fulfillment is not used in VMaaS tests. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add refresh-after-snapshot.py for refreshing OSAC on a snapshot-booted cluster using helm. Key design choices: - Start AAP and Authorino operators early (Phase 1) so they reconcile while other setup runs in parallel, saving ~4 minutes - Pre-fix fulfillment-api certificate SANs before pods start to prevent console-proxy CrashLoopBackOff from stale snapshot domain - Replace oc rollout status with wait_rollout_healthy() that detects CrashLoopBackOff and ImagePullBackOff during rollout waits - Final pod health check catches any unhealthy pods before declaring success Supporting changes: - prepare-aap.sh: skip missing deployments (BMF) instead of hard-failing - prepare-fulfillment-service.sh: add SKIP_TOKEN_CONFIG_PATCH guard to skip redundant cert/rollout work when helm already sets externalHostname - prepare-tenant.sh: accept INSTALLER_NAMESPACE from env - setup.sh: update values file paths for directory layout Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Update helm CI workflows for values directory layout - Update docs with helm deployment instructions - Gitignore helm build artifacts (charts/*/charts/*.tgz, Chart.lock) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- cluster-fulfillment-ig: add hosted. prefix to baseDomain fallback - patch-authconfig: verify SAs are in rego after replacement, fail if not - refresh-after-snapshot: use helm dependency update (matches setup.sh), fix f-string prefixes, add default to next() for clear error on missing service account client Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- instance-groups: add networkFulfillment.enabled guard to prevent duplicate resources, add empty-value guards on config rendering - patch-authconfig: add activeDeadlineSeconds, include rego content in error message for debugging - values.schema: add bmf.enabled property - refresh script: import Callable from collections.abc, fix remaining f-strings, add comment on config-as-code-ig deletion Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
When a component is disabled (e.g. bmf.enabled: false in vmaas-ci), its image config is absent from the values file. The grep pipeline failed with set -euo pipefail before the empty-check could skip it. Check for the component's presence first, then extract the tag. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The glob pattern values/*.yaml didn't match the new values/*/values.yaml directory structure, silently skipping all values file checks. Fix the glob and sync all image tags to match main's submodule commits (no submodule bump in this PR). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The sync script checked projectGitBranch in overlay kustomization files but not in helm values files. This left vmaas-ci with a mismatched projectGitBranch after the submodule bump was removed from the PR. Add projectGitBranch sync for values/*/values.yaml and fix the current mismatch. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- adopt_resources_for_helm: log warnings on annotation failures instead of silently swallowing them, so RBAC issues surface before helm upgrade - create_secrets: resolve realm.json relative to REPO_ROOT instead of process cwd, which could differ when invoked from another directory Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
omer-vishlitzky
force-pushed
the
vmaas-helm-ci
branch
from
June 20, 2026 10:24
7b9b580 to
4959e66
Compare
omer-vishlitzky
force-pushed
the
vmaas-helm-ci
branch
from
June 20, 2026 10:32
4959e66 to
32c8bdd
Compare
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
values/vmaas-ci/values.yaml (1)
100-113:⚠️ Potential issue | 🟠 MajorDuplicate top-level
bmfkey nullifies the disable flag.Lines 100–101 define
bmf.enabled: false, but lines 109–113 redefinebmf:at the top level. YAML's last-write-wins semantics for duplicate keys means the second definition overwrites the first, and the parsed result contains only theimageconfiguration. Theenabled: falseflag is lost, so BMF may deploy despite the intent to disable it in this CI overlay.Suggested fix
-# --- Bare Metal Fulfillment Operator --- -bmf: - enabled: false - -# --- Database migration --- +# --- Database migration --- dbMigrate: enabled: false image: ghcr.io/osac-project/fulfillment-service:sha-a553eee # --- Bare Metal Fulfillment Operator --- bmf: + enabled: false image: repository: ghcr.io/osac-project/bare-metal-fulfillment-operator tag: sha-0a06955 pullPolicy: Always🤖 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 `@values/vmaas-ci/values.yaml` around lines 100 - 113, The YAML file has duplicate top-level bmf keys where the first defines enabled: false and the second defines the image configuration. In YAML, duplicate keys use last-write-wins semantics, so the second bmf definition overwrites the first, causing the enabled: false flag to be lost. Merge both bmf definitions into a single bmf key that contains both the enabled property set to false and the nested image configuration with repository, tag, and pullPolicy properties.
♻️ Duplicate comments (1)
charts/osac/values.schema.json (1)
485-508:⚠️ Potential issue | 🟠 Major | ⚡ Quick winTighten schema for
instanceGroups/clusterFulfillmentto match consumed keys.Line 491–507 leaves nested objects largely unstructured, but templates consume concrete keys (
instanceGroups.cluster.config,instanceGroups.network.config, andclusterFulfillment.secretincharts/osac/templates/cluster-fulfillment-ig.yaml). This weakens validation and allows typo-driven misconfigurations to pass schema checks.Suggested schema tightening
"instanceGroups": { "type": "object", "properties": { "enabled": { - "type": "boolean" + "type": "boolean", + "default": true }, "cluster": { - "type": "object" + "type": "object", + "properties": { + "config": { + "type": "object" + } + } }, "network": { - "type": "object" + "type": "object", + "properties": { + "config": { + "type": "object" + } + } } } }, "clusterFulfillment": { "type": "object", "properties": { "enabled": { "type": "boolean" }, "config": { "type": "object" + }, + "secret": { + "type": "object" } } }🤖 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 `@charts/osac/values.schema.json` around lines 485 - 508, The instanceGroups and clusterFulfillment object definitions in the schema are too permissive and allow any properties. Replace the generic "type": "object" definitions with explicit property schemas that match what is actually consumed by the templates. For instanceGroups.cluster and instanceGroups.network, add properties objects that define the concrete keys like config. For clusterFulfillment, add the secret property that is referenced in the templates along with the existing config property. This will tighten validation and catch typos or misconfigurations that deviate from the expected structure.
🤖 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 `@charts/osac/templates/cluster-fulfillment-ig.yaml`:
- Line 5: The $baseDomain variable on line 5 always formats hosted.%s without
validating that $ingressDomain is not empty, causing an invalid domain like
hosted. to be set when lookup returns empty (common in helm template or CI
dry-runs). Add a guard condition to check if $ingressDomain is not empty before
applying the printf and trimPrefix operations to compose $baseDomain, ensuring
line 20 never emits an invalid domain value.
In `@charts/osac/templates/hooks/patch-authconfig.yaml`:
- Around line 28-33: The patch container in the hook lacks security hardening
and resource constraints. Add a securityContext section to the container
specification (within the patch container definition) that sets runAsNonRoot to
true, readOnlyRootFilesystem to true, allowPrivilegeEscalation to false, and
includes a capabilities drop of ALL. Additionally, add a resources section
specifying both requests and limits for CPU and memory to ensure proper pod
scheduling and resource bounds as per Kubernetes security guidelines.
- Around line 44-45: Update the EXTRA_SAS list in the patch-authconfig.yaml hook
to use the correct ServiceAccount name. Change the EXTRA_SAS variable definition
from including "osac-operator" to "osac-operator-controller-manager" to align
with the service account name expected by sync-authconfig-rego.py and match the
documented behavior. This ensures the operator controller manager ServiceAccount
receives the required emergency admin access for gRPC sync operations.
In `@scripts/refresh-after-snapshot.py`:
- Around line 357-364: In the _try_apply_pool function, add the parameter
check=False to the subprocess.run call to explicitly indicate that the function
is intentionally checking the return code manually rather than letting
subprocess raise an exception. This clarifies the intent and satisfies the Ruff
PLW1510 linting rule.
- Around line 554-572: The base_domain calculation in the upgrade_osac function
uses removeprefix("apps.") which silently fails to remove the prefix if
cluster_domain doesn't start with "apps.", producing an incorrect domain. Add a
guard condition to check if cluster_domain actually starts with "apps." before
attempting to remove it. If it does, remove the prefix; if it doesn't, extract
the domain suffix more robustly (e.g., by splitting on the first dot after
"hosted." or by using string manipulation to get the expected format). This
ensures base_domain is constructed correctly regardless of what prefix the
cluster_domain has.
In `@scripts/sync-image-tags.sh`:
- Around line 125-138: The aap_full_commit variable is being computed inside the
values_file loop at line 126, causing redundant git submodule status queries on
each iteration. Move the aap_full_commit computation (the git -C "${REPO_ROOT}"
submodule status base/osac-aap command) outside and before the for values_file
loop begins, following the same pattern as the image tag computations on lines
75-78. After moving, the variable will be computed once and can be reused for
all iterations of the loop.
---
Outside diff comments:
In `@values/vmaas-ci/values.yaml`:
- Around line 100-113: The YAML file has duplicate top-level bmf keys where the
first defines enabled: false and the second defines the image configuration. In
YAML, duplicate keys use last-write-wins semantics, so the second bmf definition
overwrites the first, causing the enabled: false flag to be lost. Merge both bmf
definitions into a single bmf key that contains both the enabled property set to
false and the nested image configuration with repository, tag, and pullPolicy
properties.
---
Duplicate comments:
In `@charts/osac/values.schema.json`:
- Around line 485-508: The instanceGroups and clusterFulfillment object
definitions in the schema are too permissive and allow any properties. Replace
the generic "type": "object" definitions with explicit property schemas that
match what is actually consumed by the templates. For instanceGroups.cluster and
instanceGroups.network, add properties objects that define the concrete keys
like config. For clusterFulfillment, add the secret property that is referenced
in the templates along with the existing config property. This will tighten
validation and catch typos or misconfigurations that deviate from the expected
structure.
🪄 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: osac-project/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 980585d0-cd4b-473b-9bd5-de711f31fbe1
📒 Files selected for processing (24)
.github/workflows/helm-integration.yaml.github/workflows/helm-lint.yaml.gitignoreREADME.mdcharts/osac/Chart.yamlcharts/osac/templates/cluster-fulfillment-ig.yamlcharts/osac/templates/hooks/patch-authconfig.yamlcharts/osac/templates/hooks/pre-install-validate.yamlcharts/osac/templates/hub-access.yamlcharts/osac/templates/instance-groups.yamlcharts/osac/templates/operator-rbac-supplement.yamlcharts/osac/values.schema.jsoncharts/osac/values.yamldocs/helm-deployment-guide.mddocs/network-backend.mdscripts/prepare-aap.shscripts/prepare-fulfillment-service.shscripts/prepare-tenant.shscripts/refresh-after-snapshot.pyscripts/setup.shscripts/sync-image-tags.shvalues/caas-ci/values.yamlvalues/development/values.yamlvalues/vmaas-ci/values.yaml
Comment on lines
+44
to
+45
| EXTRA_SAS = ["template-publisher", "osac-operator"] | ||
| TIMEOUT = 300 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Check emergency SA identities and operator SA names across chart/scripts/docs..."
rg -n -C2 'EXTRA_SAS|EXTRA_EMERGENCY_SAS|emergency_service_accounts|osac-operator-controller-manager|osac-operator\b|serviceAccountName:' charts/osac scripts README.md docs || true
echo
echo "Focus specifically on hook and operator templates:"
rg -n -C3 'patch-authconfig|serviceAccountName|osac-operator-controller-manager|osac-operator\b' charts/osac/templates || trueRepository: osac-project/osac-installer
Length of output: 17690
🏁 Script executed:
fd -type f -name "serviceaccount*" "base/osac-operator" 2>/dev/null | head -20Repository: osac-project/osac-installer
Length of output: 53
🏁 Script executed:
find base/osac-operator -type f \( -name "*.yaml" -o -name "*.yml" \) -exec grep -l "kind: ServiceAccount" {} \;Repository: osac-project/osac-installer
Length of output: 53
🏁 Script executed:
grep -r "serviceAccountName" base/osac-operator/charts --include="*.yaml" --include="*.yml" | head -20Repository: osac-project/osac-installer
Length of output: 130
🏁 Script executed:
ls -la base/ 2>/dev/null || echo "base directory structure:"
find base -maxdepth 2 -type d 2>/dev/null | head -20Repository: osac-project/osac-installer
Length of output: 984
🏁 Script executed:
# Check if submodules are initialized
git config --file .gitmodules --name-only --get-regexp path | head -10Repository: osac-project/osac-installer
Length of output: 231
🏁 Script executed:
# Look at Chart.yaml dependencies to understand operator chart reference
cat charts/osac/Chart.yamlRepository: osac-project/osac-installer
Length of output: 1119
🏁 Script executed:
# Search for all ServiceAccount definitions in the codebase
rg "kind: ServiceAccount" --type yaml -A 2 | head -40Repository: osac-project/osac-installer
Length of output: 2164
🏁 Script executed:
# Look at the operator-rbac-supplement more carefully
cat charts/osac/templates/operator-rbac-supplement.yamlRepository: osac-project/osac-installer
Length of output: 1285
🏁 Script executed:
# Check if there are references to both SA names in the hook or related code
cat charts/osac/templates/hooks/patch-authconfig.yamlRepository: osac-project/osac-installer
Length of output: 3282
🏁 Script executed:
# Let me check what the actual Deployment name is and if it has a serviceAccountName
rg "osac-operator-controller-manager|serviceAccountName" charts/osac --type yaml -B 2 -A 2Repository: osac-project/osac-installer
Length of output: 848
🏁 Script executed:
# Check the sync script more carefully to understand the context
cat scripts/sync-authconfig-rego.pyRepository: osac-project/osac-installer
Length of output: 5580
Update hook to use osac-operator-controller-manager in EXTRA_SAS (line 44).
The hook currently injects "osac-operator" into the AuthConfig emergency service accounts, but sync-authconfig-rego.py (line 33) and the README documentation use "osac-operator-controller-manager". This mismatch means the actual operator ServiceAccount will not receive the required emergency admin access for gRPC sync operations.
Current hook (incorrect)
EXTRA_SAS = ["template-publisher", "osac-operator"]
Change to:
EXTRA_SAS = ["template-publisher", "osac-operator-controller-manager"]
🤖 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 `@charts/osac/templates/hooks/patch-authconfig.yaml` around lines 44 - 45,
Update the EXTRA_SAS list in the patch-authconfig.yaml hook to use the correct
ServiceAccount name. Change the EXTRA_SAS variable definition from including
"osac-operator" to "osac-operator-controller-manager" to align with the service
account name expected by sync-authconfig-rego.py and match the documented
behavior. This ensures the operator controller manager ServiceAccount receives
the required emergency admin access for gRPC sync operations.
Comment on lines
+357
to
+364
| def _try_apply_pool() -> bool: | ||
| r = subprocess.run( | ||
| ["oc", "apply", "-f", "-"], | ||
| input=pool_yaml, text=True, capture_output=True, cwd=str(REPO_ROOT), | ||
| ) | ||
| if r.returncode != 0: | ||
| print(f" MetalLB apply failed (retrying): {r.stderr.strip()}", file=sys.stderr) | ||
| return r.returncode == 0 |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | 💤 Low value
Add explicit check=False for clarity.
Ruff PLW1510 flags this subprocess.run call as missing an explicit check argument. Since the return code is intentionally checked manually, adding check=False makes the intent clear.
Suggested fix
r = subprocess.run(
["oc", "apply", "-f", "-"],
- input=pool_yaml, text=True, capture_output=True, cwd=str(REPO_ROOT),
+ input=pool_yaml, text=True, capture_output=True, cwd=str(REPO_ROOT), check=False,
)🧰 Tools
🪛 ast-grep (0.43.0)
[error] 357-360: Command coming from incoming request
Context: subprocess.run(
["oc", "apply", "-f", "-"],
input=pool_yaml, text=True, capture_output=True, cwd=str(REPO_ROOT),
)
Note: [CWE-20].
(subprocess-from-request)
🪛 Ruff (0.15.17)
[warning] 358-358: subprocess.run without explicit check argument
Add explicit check=False
(PLW1510)
[error] 359-359: Starting a process with a partial executable path
(S607)
🤖 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 `@scripts/refresh-after-snapshot.py` around lines 357 - 364, In the
_try_apply_pool function, add the parameter check=False to the subprocess.run
call to explicitly indicate that the function is intentionally checking the
return code manually rather than letting subprocess raise an exception. This
clarifies the intent and satisfies the Ruff PLW1510 linting rule.
Source: Linters/SAST tools
Comment on lines
+554
to
+572
| def upgrade_osac(config: RefreshConfig) -> None: | ||
| print(" Upgrading osac chart...") | ||
| (REPO_ROOT / "charts/osac/Chart.lock").unlink(missing_ok=True) | ||
| run(["helm", "dependency", "build", "charts/osac/"]) | ||
| adopt_resources_for_helm(config) | ||
| # Delete stale config-as-code-ig so helm recreates it from chart values. | ||
| # The AAP subchart manages this secret; deleting forces a fresh render. | ||
| oc("delete", "secret", "config-as-code-ig", "-n", config.namespace, | ||
| "--ignore-not-found") | ||
| base_domain = "hosted." + config.cluster_domain.removeprefix("apps.") | ||
| run(["helm", "upgrade", "osac", "charts/osac/", | ||
| "--namespace", config.namespace, | ||
| "--values", config.values_file, | ||
| "--set", f"service.externalHostname={config.external_host}", | ||
| "--set", f"service.internalHostname={config.internal_host}", | ||
| "--set", "aap.bootstrap.enabled=false", | ||
| "--set", f"clusterFulfillment.config.HOSTED_CLUSTER_BASE_DOMAIN={base_domain}", | ||
| "--timeout", "15m"]) | ||
|
|
There was a problem hiding this comment.
removeprefix("apps.") silently produces wrong base_domain if cluster domain has a different prefix.
If cluster_domain is not prefixed with apps. (e.g., custom.example.com), removeprefix returns the original string unchanged, yielding hosted.custom.example.com instead of the expected hosted.example.com.
Consider adding a guard or extracting the suffix more robustly:
Suggested fix
- base_domain = "hosted." + config.cluster_domain.removeprefix("apps.")
+ if not config.cluster_domain.startswith("apps."):
+ raise RuntimeError(
+ f"Unexpected cluster domain format: {config.cluster_domain} (expected apps.<base>)")
+ base_domain = "hosted." + config.cluster_domain.removeprefix("apps.")🤖 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 `@scripts/refresh-after-snapshot.py` around lines 554 - 572, The base_domain
calculation in the upgrade_osac function uses removeprefix("apps.") which
silently fails to remove the prefix if cluster_domain doesn't start with
"apps.", producing an incorrect domain. Add a guard condition to check if
cluster_domain actually starts with "apps." before attempting to remove it. If
it does, remove the prefix; if it doesn't, extract the domain suffix more
robustly (e.g., by splitting on the first dot after "hosted." or by using string
manipulation to get the expected format). This ensures base_domain is
constructed correctly regardless of what prefix the cluster_domain has.
Comment on lines
+125
to
+138
| # Sync projectGitBranch (full 40-char commit) with osac-aap submodule. | ||
| aap_full_commit=$(git -C "${REPO_ROOT}" submodule status base/osac-aap | awk '{print $1}' | tr -d ' +-') | ||
| grep -q "projectGitBranch:" "${values_file}" || continue | ||
| current_branch=$(grep "projectGitBranch:" "${values_file}" | head -1 | sed 's/.*projectGitBranch: *"\{0,1\}\([^"]*\)"\{0,1\}/\1/') | ||
| [[ -z "${current_branch}" ]] && continue | ||
| if [[ "${current_branch}" == "${aap_full_commit}" ]]; then | ||
| echo "${name} projectGitBranch: OK" | ||
| elif [[ "${1:-}" == "--fix" ]]; then | ||
| sed -i "s|projectGitBranch: .*|projectGitBranch: \"${aap_full_commit}\"|" "${values_file}" | ||
| echo "${name} projectGitBranch: FIXED ${current_branch} -> ${aap_full_commit}" | ||
| else | ||
| echo "${name} projectGitBranch: MISMATCH current=${current_branch} expected=${aap_full_commit}" | ||
| errors=$((errors + 1)) | ||
| fi |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Move aap_full_commit computation outside the loop for consistency and efficiency.
Lines 75–78 compute image tags once outside the for values_file loop. The aap_full_commit (line 126) should follow the same pattern: compute once before the loop, then reuse. This avoids redundant git submodule queries and keeps the pattern consistent.
♻️ Proposed refactor
aap_tag="sha-$(git -C "${REPO_ROOT}" submodule status base/osac-aap | awk '{print $1}' | tr -d ' +-' | cut -c1-7)"
bmf_tag="sha-$(git -C "${REPO_ROOT}" submodule status base/bare-metal-fulfillment-operator | awk '{print $1}' | tr -d ' +-' | cut -c1-7)"
+aap_full_commit=$(git -C "${REPO_ROOT}" submodule status base/osac-aap | awk '{print $1}' | tr -d ' +-')
for values_file in "${REPO_ROOT}"/values/*/values.yaml; do
[[ ! -f "${values_file}" ]] && continue
name=$(basename "$(dirname "${values_file}")")
@@ -123,7 +126,6 @@
done
# Sync projectGitBranch (full 40-char commit) with osac-aap submodule.
- aap_full_commit=$(git -C "${REPO_ROOT}" submodule status base/osac-aap | awk '{print $1}' | tr -d ' +-')
grep -q "projectGitBranch:" "${values_file}" || continue📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| # Sync projectGitBranch (full 40-char commit) with osac-aap submodule. | |
| aap_full_commit=$(git -C "${REPO_ROOT}" submodule status base/osac-aap | awk '{print $1}' | tr -d ' +-') | |
| grep -q "projectGitBranch:" "${values_file}" || continue | |
| current_branch=$(grep "projectGitBranch:" "${values_file}" | head -1 | sed 's/.*projectGitBranch: *"\{0,1\}\([^"]*\)"\{0,1\}/\1/') | |
| [[ -z "${current_branch}" ]] && continue | |
| if [[ "${current_branch}" == "${aap_full_commit}" ]]; then | |
| echo "${name} projectGitBranch: OK" | |
| elif [[ "${1:-}" == "--fix" ]]; then | |
| sed -i "s|projectGitBranch: .*|projectGitBranch: \"${aap_full_commit}\"|" "${values_file}" | |
| echo "${name} projectGitBranch: FIXED ${current_branch} -> ${aap_full_commit}" | |
| else | |
| echo "${name} projectGitBranch: MISMATCH current=${current_branch} expected=${aap_full_commit}" | |
| errors=$((errors + 1)) | |
| fi | |
| # Sync projectGitBranch (full 40-char commit) with osac-aap submodule. | |
| grep -q "projectGitBranch:" "${values_file}" || continue | |
| current_branch=$(grep "projectGitBranch:" "${values_file}" | head -1 | sed 's/.*projectGitBranch: *"\{0,1\}\([^"]*\)"\{0,1\}/\1/') | |
| [[ -z "${current_branch}" ]] && continue | |
| if [[ "${current_branch}" == "${aap_full_commit}" ]]; then | |
| echo "${name} projectGitBranch: OK" | |
| elif [[ "${1:-}" == "--fix" ]]; then | |
| sed -i "s|projectGitBranch: .*|projectGitBranch: \"${aap_full_commit}\"|" "${values_file}" | |
| echo "${name} projectGitBranch: FIXED ${current_branch} -> ${aap_full_commit}" | |
| else | |
| echo "${name} projectGitBranch: MISMATCH current=${current_branch} expected=${aap_full_commit}" | |
| errors=$((errors + 1)) | |
| fi |
🤖 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 `@scripts/sync-image-tags.sh` around lines 125 - 138, The aap_full_commit
variable is being computed inside the values_file loop at line 126, causing
redundant git submodule status queries on each iteration. Move the
aap_full_commit computation (the git -C "${REPO_ROOT}" submodule status
base/osac-aap command) outside and before the for values_file loop begins,
following the same pattern as the image tag computations on lines 75-78. After
moving, the variable will be computed once and can be reused for all iterations
of the loop.
Project sync can trigger AAP reconciliation that restarts the gateway. Without waiting, publish-templates immediately hits 502s and times out. Adds ~1s in the happy path (single curl check), prevents ~5min timeout in the failure path. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- cluster-fulfillment-ig: skip baseDomain computation when lookup returns empty (helm template / CI dry-runs), avoids invalid "hosted." - patch-authconfig: add securityContext (drop ALL capabilities, readOnlyRootFilesystem, no privilege escalation) and resource requests/limits Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The osac-operator now registers a BareMetalInstance feedback controller (OSAC-1352) that requires the BMF CRDs to exist for cache sync. With bmfCrds gated behind bmf.enabled, the CRDs were missing when BMF was disabled, causing the operator to CrashLoopBackOff on startup. Keep bmfCrds always installed (CRDs only, no pods) while the BMF operator deployment remains gated behind bmf.enabled. Also add check_all_pods after Phase 4 — post-flight restarts the operator (prepare-aap sets OSAC_AAP_URL env), so crashes that happen after Phase 3's health check are now caught before reporting success. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
omer-vishlitzky
force-pushed
the
vmaas-helm-ci
branch
from
June 20, 2026 16:58
314f252 to
795b9bc
Compare
MetalLB webhook endpoint wait timed out on iteration 3 of stress tests because OLM was busy reconciling AAP/Authorino CSV changes in parallel. Increase the timeout from 120s to 300s to accommodate OLM contention. Add a post-install/post-upgrade hook that patches the grpc-server deployment probes (timeoutSeconds: 5, failureThreshold: 5). The exec- based readiness probe spawns a full process; under CPU/IO pressure from AAP automation jobs on single-node clusters, process startup exceeds the default 1s timeout, causing the headless Service to drop the pod IP and Envoy to return "no healthy upstream". Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
3 tasks
openshift-merge-bot Bot
pushed a commit
to openshift/release
that referenced
this pull request
Jun 21, 2026
* Switch OSAC CI from kustomize to helm-based refresh Replace the kustomize-based refresh flow with the Python helm refresh script (refresh-after-snapshot.py). The boot step now uses the vmaas-helm snapshot flavor and deploys via helm upgrade instead of oc apply -k. Changes: - Boot ref: E2E_VALUES_FILE replaces E2E_KUSTOMIZE_OVERLAY, flavor defaults to vmaas-helm - Boot script: license mounts to values dir, component overrides use sed on helm values YAML (handles both image:tag and split repository/tag formats), refresh runs python3 -u refresh-after-snapshot.py - osac-installer config: add values/ to run_if_changed trigger - osac-test-infra config: bump vmaas periodic to hourly, drop kustomize full-setup job Depends on osac-project/osac-installer#296 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Fix clone name mismatch, operator override, and AAP verification - Update CLONE_NAME to vmaas-helm in test and destroy steps to match the boot step — prevents kubeconfig-not-found and VM leak - Fix split repository/tag component override to also replace the repository line (not just the tag) so osac-operator PRs get the correct CI registry image - Restore grep -q verification after AAP sed overrides to fail fast if the field name changes in the values file Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Add grep verification for component image override Fail fast if the sed replacement didn't match anything in the values file, instead of silently testing against the pinned image. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * make jobs * Add storage, MCO, and subnet pod diagnostics to gather script Collect LVMS/topolvm state (pods, events, logs, LVMCluster, VolumeAttachments), MachineConfig status and pull-secret registry list, CDI importer pod logs from subnet namespaces, and service account imagePullSecrets state. These were missing when debugging CI failures involving volume mount timeouts and image pull errors. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Mount pull secret into installer container for namespace secret creation The old kustomize flow created a quay-pull-secret in the OSAC namespace via secretGenerator, giving AAP job pods credentials to pull CI-built EE images. The helm migration dropped this mount, causing intermittent ErrImagePull when the MCO hasn't propagated the global pull secret to CRI-O before AAP launches automation jobs. Re-add the pull secret as a volume mount at /installer/pull-secret.json so refresh-after-snapshot.py can create the namespace-level secret. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Increase helm full install periodic frequency to every 4 hours Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
11 tasks
krisnababu
pushed a commit
to krisnababu/release
that referenced
this pull request
Jun 29, 2026
* Switch OSAC CI from kustomize to helm-based refresh Replace the kustomize-based refresh flow with the Python helm refresh script (refresh-after-snapshot.py). The boot step now uses the vmaas-helm snapshot flavor and deploys via helm upgrade instead of oc apply -k. Changes: - Boot ref: E2E_VALUES_FILE replaces E2E_KUSTOMIZE_OVERLAY, flavor defaults to vmaas-helm - Boot script: license mounts to values dir, component overrides use sed on helm values YAML (handles both image:tag and split repository/tag formats), refresh runs python3 -u refresh-after-snapshot.py - osac-installer config: add values/ to run_if_changed trigger - osac-test-infra config: bump vmaas periodic to hourly, drop kustomize full-setup job Depends on osac-project/osac-installer#296 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Fix clone name mismatch, operator override, and AAP verification - Update CLONE_NAME to vmaas-helm in test and destroy steps to match the boot step — prevents kubeconfig-not-found and VM leak - Fix split repository/tag component override to also replace the repository line (not just the tag) so osac-operator PRs get the correct CI registry image - Restore grep -q verification after AAP sed overrides to fail fast if the field name changes in the values file Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Add grep verification for component image override Fail fast if the sed replacement didn't match anything in the values file, instead of silently testing against the pinned image. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * make jobs * Add storage, MCO, and subnet pod diagnostics to gather script Collect LVMS/topolvm state (pods, events, logs, LVMCluster, VolumeAttachments), MachineConfig status and pull-secret registry list, CDI importer pod logs from subnet namespaces, and service account imagePullSecrets state. These were missing when debugging CI failures involving volume mount timeouts and image pull errors. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Mount pull secret into installer container for namespace secret creation The old kustomize flow created a quay-pull-secret in the OSAC namespace via secretGenerator, giving AAP job pods credentials to pull CI-built EE images. The helm migration dropped this mount, causing intermittent ErrImagePull when the MCO hasn't propagated the global pull secret to CRI-O before AAP launches automation jobs. Re-add the pull secret as a volume mount at /installer/pull-secret.json so refresh-after-snapshot.py can create the namespace-level secret. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Increase helm full install periodic frequency to every 4 hours Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
krisnababu
pushed a commit
to oharan2/release
that referenced
this pull request
Jul 3, 2026
* Switch OSAC CI from kustomize to helm-based refresh Replace the kustomize-based refresh flow with the Python helm refresh script (refresh-after-snapshot.py). The boot step now uses the vmaas-helm snapshot flavor and deploys via helm upgrade instead of oc apply -k. Changes: - Boot ref: E2E_VALUES_FILE replaces E2E_KUSTOMIZE_OVERLAY, flavor defaults to vmaas-helm - Boot script: license mounts to values dir, component overrides use sed on helm values YAML (handles both image:tag and split repository/tag formats), refresh runs python3 -u refresh-after-snapshot.py - osac-installer config: add values/ to run_if_changed trigger - osac-test-infra config: bump vmaas periodic to hourly, drop kustomize full-setup job Depends on osac-project/osac-installer#296 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Fix clone name mismatch, operator override, and AAP verification - Update CLONE_NAME to vmaas-helm in test and destroy steps to match the boot step — prevents kubeconfig-not-found and VM leak - Fix split repository/tag component override to also replace the repository line (not just the tag) so osac-operator PRs get the correct CI registry image - Restore grep -q verification after AAP sed overrides to fail fast if the field name changes in the values file Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Add grep verification for component image override Fail fast if the sed replacement didn't match anything in the values file, instead of silently testing against the pinned image. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * make jobs * Add storage, MCO, and subnet pod diagnostics to gather script Collect LVMS/topolvm state (pods, events, logs, LVMCluster, VolumeAttachments), MachineConfig status and pull-secret registry list, CDI importer pod logs from subnet namespaces, and service account imagePullSecrets state. These were missing when debugging CI failures involving volume mount timeouts and image pull errors. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Mount pull secret into installer container for namespace secret creation The old kustomize flow created a quay-pull-secret in the OSAC namespace via secretGenerator, giving AAP job pods credentials to pull CI-built EE images. The helm migration dropped this mount, causing intermittent ErrImagePull when the MCO hasn't propagated the global pull secret to CRI-O before AAP launches automation jobs. Re-add the pull secret as a volume mount at /installer/pull-secret.json so refresh-after-snapshot.py can create the namespace-level secret. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Increase helm full install periodic frequency to every 4 hours Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
andrej1991
pushed a commit
to andrej1991/release
that referenced
this pull request
Jul 6, 2026
* Switch OSAC CI from kustomize to helm-based refresh Replace the kustomize-based refresh flow with the Python helm refresh script (refresh-after-snapshot.py). The boot step now uses the vmaas-helm snapshot flavor and deploys via helm upgrade instead of oc apply -k. Changes: - Boot ref: E2E_VALUES_FILE replaces E2E_KUSTOMIZE_OVERLAY, flavor defaults to vmaas-helm - Boot script: license mounts to values dir, component overrides use sed on helm values YAML (handles both image:tag and split repository/tag formats), refresh runs python3 -u refresh-after-snapshot.py - osac-installer config: add values/ to run_if_changed trigger - osac-test-infra config: bump vmaas periodic to hourly, drop kustomize full-setup job Depends on osac-project/osac-installer#296 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Fix clone name mismatch, operator override, and AAP verification - Update CLONE_NAME to vmaas-helm in test and destroy steps to match the boot step — prevents kubeconfig-not-found and VM leak - Fix split repository/tag component override to also replace the repository line (not just the tag) so osac-operator PRs get the correct CI registry image - Restore grep -q verification after AAP sed overrides to fail fast if the field name changes in the values file Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Add grep verification for component image override Fail fast if the sed replacement didn't match anything in the values file, instead of silently testing against the pinned image. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * make jobs * Add storage, MCO, and subnet pod diagnostics to gather script Collect LVMS/topolvm state (pods, events, logs, LVMCluster, VolumeAttachments), MachineConfig status and pull-secret registry list, CDI importer pod logs from subnet namespaces, and service account imagePullSecrets state. These were missing when debugging CI failures involving volume mount timeouts and image pull errors. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Mount pull secret into installer container for namespace secret creation The old kustomize flow created a quay-pull-secret in the OSAC namespace via secretGenerator, giving AAP job pods credentials to pull CI-built EE images. The helm migration dropped this mount, causing intermittent ErrImagePull when the MCO hasn't propagated the global pull secret to CRI-O before AAP launches automation jobs. Re-add the pull secret as a volume mount at /installer/pull-secret.json so refresh-after-snapshot.py can create the namespace-level secret. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Increase helm full install periodic frequency to every 4 hours Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
TimurMP
pushed a commit
to TimurMP/release
that referenced
this pull request
Jul 8, 2026
* Switch OSAC CI from kustomize to helm-based refresh Replace the kustomize-based refresh flow with the Python helm refresh script (refresh-after-snapshot.py). The boot step now uses the vmaas-helm snapshot flavor and deploys via helm upgrade instead of oc apply -k. Changes: - Boot ref: E2E_VALUES_FILE replaces E2E_KUSTOMIZE_OVERLAY, flavor defaults to vmaas-helm - Boot script: license mounts to values dir, component overrides use sed on helm values YAML (handles both image:tag and split repository/tag formats), refresh runs python3 -u refresh-after-snapshot.py - osac-installer config: add values/ to run_if_changed trigger - osac-test-infra config: bump vmaas periodic to hourly, drop kustomize full-setup job Depends on osac-project/osac-installer#296 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Fix clone name mismatch, operator override, and AAP verification - Update CLONE_NAME to vmaas-helm in test and destroy steps to match the boot step — prevents kubeconfig-not-found and VM leak - Fix split repository/tag component override to also replace the repository line (not just the tag) so osac-operator PRs get the correct CI registry image - Restore grep -q verification after AAP sed overrides to fail fast if the field name changes in the values file Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Add grep verification for component image override Fail fast if the sed replacement didn't match anything in the values file, instead of silently testing against the pinned image. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * make jobs * Add storage, MCO, and subnet pod diagnostics to gather script Collect LVMS/topolvm state (pods, events, logs, LVMCluster, VolumeAttachments), MachineConfig status and pull-secret registry list, CDI importer pod logs from subnet namespaces, and service account imagePullSecrets state. These were missing when debugging CI failures involving volume mount timeouts and image pull errors. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Mount pull secret into installer container for namespace secret creation The old kustomize flow created a quay-pull-secret in the OSAC namespace via secretGenerator, giving AAP job pods credentials to pull CI-built EE images. The helm migration dropped this mount, causing intermittent ErrImagePull when the MCO hasn't propagated the global pull secret to CRI-O before AAP launches automation jobs. Re-add the pull secret as a volume mount at /installer/pull-secret.json so refresh-after-snapshot.py can create the namespace-level secret. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Increase helm full install periodic frequency to every 4 hours Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
SachinNinganure
pushed a commit
to SachinNinganure/release
that referenced
this pull request
Jul 20, 2026
* Switch OSAC CI from kustomize to helm-based refresh Replace the kustomize-based refresh flow with the Python helm refresh script (refresh-after-snapshot.py). The boot step now uses the vmaas-helm snapshot flavor and deploys via helm upgrade instead of oc apply -k. Changes: - Boot ref: E2E_VALUES_FILE replaces E2E_KUSTOMIZE_OVERLAY, flavor defaults to vmaas-helm - Boot script: license mounts to values dir, component overrides use sed on helm values YAML (handles both image:tag and split repository/tag formats), refresh runs python3 -u refresh-after-snapshot.py - osac-installer config: add values/ to run_if_changed trigger - osac-test-infra config: bump vmaas periodic to hourly, drop kustomize full-setup job Depends on osac-project/osac-installer#296 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Fix clone name mismatch, operator override, and AAP verification - Update CLONE_NAME to vmaas-helm in test and destroy steps to match the boot step — prevents kubeconfig-not-found and VM leak - Fix split repository/tag component override to also replace the repository line (not just the tag) so osac-operator PRs get the correct CI registry image - Restore grep -q verification after AAP sed overrides to fail fast if the field name changes in the values file Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Add grep verification for component image override Fail fast if the sed replacement didn't match anything in the values file, instead of silently testing against the pinned image. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * make jobs * Add storage, MCO, and subnet pod diagnostics to gather script Collect LVMS/topolvm state (pods, events, logs, LVMCluster, VolumeAttachments), MachineConfig status and pull-secret registry list, CDI importer pod logs from subnet namespaces, and service account imagePullSecrets state. These were missing when debugging CI failures involving volume mount timeouts and image pull errors. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Mount pull secret into installer container for namespace secret creation The old kustomize flow created a quay-pull-secret in the OSAC namespace via secretGenerator, giving AAP job pods credentials to pull CI-built EE images. The helm migration dropped this mount, causing intermittent ErrImagePull when the MCO hasn't propagated the global pull secret to CRI-O before AAP launches automation jobs. Re-add the pull secret as a volume mount at /installer/pull-secret.json so refresh-after-snapshot.py can create the namespace-level secret. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Increase helm full install periodic frequency to every 4 hours Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
scripts/refresh-after-snapshot-helm.py— a Python refresh script that deploys OSAC viahelm upgradeinstead ofkustomize applyon snapshot-based CI clustersvalues/vmaas-ci/values.yaml)INSTALLER_NAMESPACEenv var for Helm mode🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
Improvements
Configuration Changes