trusted-execution-clusters: Enable full suite - #82662
Conversation
Use native OpenShift as Trustee platform, attest Azure VM against it. Optional test. Signed-off-by: Jakob Naucke <jnaucke@redhat.com> Assisted-by: Claude Opus 4.6
|
Skipping CI for Draft Pull Request. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Jakob-Naucke 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 |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
💤 Files with no reviewable changes (2)
WalkthroughAdds operator CI configurations and wires an Azure integration test reference that provisions tooling and cert-manager before repeatedly running integration tests. ChangesAzure CI integration
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant CI as CI configuration
participant Ref as Azure integration ref
participant Script as Commands script
participant Azure as Azure CLI
participant Cluster as OpenShift cluster
participant Tests as Integration tests
CI->>Ref: Select azure-test-image and test ref
Ref->>Script: Execute Azure test commands
Script->>Azure: Login with service-principal credentials
Script->>Cluster: Apply cert-manager manifest
Script->>Tests: Run make integration-tests repeatedly
🚥 Pre-merge checks | ✅ 14 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (14 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 |
|
/pj-rehearse |
|
@Jakob-Naucke: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
ci-operator/step-registry/trusted-execution-clusters/ref/azure-integration-test/trusted-execution-clusters-ref-azure-integration-test-commands.sh (1)
10-10: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winPin the Azure CLI dependency.
pip install azure-cliresolves the package and all transitive dependencies at job runtime, allowing unrelated releases to change or break this test. Use a reviewed constraint/lock file with hashes or bake the pinned version intoazure-test-image.🤖 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 `@ci-operator/step-registry/trusted-execution-clusters/ref/azure-integration-test/trusted-execution-clusters-ref-azure-integration-test-commands.sh` at line 10, Update the Azure CLI installation command in the trusted execution cluster test setup to use a reviewed pinned dependency source, such as a constraints/lock file with hashes, or replace the runtime install with the pinned Azure CLI version from azure-test-image. Ensure transitive dependencies are reproducibly constrained rather than resolved freely at job runtime.
🤖 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
`@ci-operator/step-registry/trusted-execution-clusters/ref/azure-integration-test/trusted-execution-clusters-ref-azure-integration-test-commands.sh`:
- Around line 40-43: Update the attestation test execution around the make
attestation-tests loop to avoid unbounded retries and ensure a final test
failure makes the step exit non-zero. Prefer running make attestation-tests once
for normal CI, or implement a bounded retry with an explicit failure exit after
the retry limit.
- Around line 28-30: Remove the `git remote add test`, `git fetch test`, and
`git switch timed-client-ci` overrides so normal CI runs the reviewed checkout;
also remove the mutable personal Quay image override at the corresponding
image-selection symbols. If debugging support is required, isolate it behind an
explicit manual path and pin the source revision by commit and image by digest.
- Line 5: Replace the direct curl-to-shell invocation with a pinned or prebuilt
Rust toolchain in the trusted execution setup. If retaining the installer,
download it to a local file, verify its reviewed checksum or signature, and only
then execute it; ensure no unverified remote content runs with access to Azure
secrets.
---
Nitpick comments:
In
`@ci-operator/step-registry/trusted-execution-clusters/ref/azure-integration-test/trusted-execution-clusters-ref-azure-integration-test-commands.sh`:
- Line 10: Update the Azure CLI installation command in the trusted execution
cluster test setup to use a reviewed pinned dependency source, such as a
constraints/lock file with hashes, or replace the runtime install with the
pinned Azure CLI version from azure-test-image. Ensure transitive dependencies
are reproducibly constrained rather than resolved freely at job runtime.
🪄 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: Pro Plus
Run ID: 77d53dd0-d4ab-4b7e-a12b-3bbd7a72d084
⛔ Files ignored due to path filters (1)
ci-operator/jobs/trusted-execution-clusters/operator/trusted-execution-clusters-operator-main-presubmits.yamlis excluded by!ci-operator/jobs/**
📒 Files selected for processing (6)
ci-operator/config/trusted-execution-clusters/operator/trusted-execution-clusters-operator-main.yamlci-operator/config/trusted-execution-clusters/operator/trusted-execution-clusters-operator-main__azure.yamlci-operator/step-registry/trusted-execution-clusters/ref/azure-integration-test/OWNERSci-operator/step-registry/trusted-execution-clusters/ref/azure-integration-test/trusted-execution-clusters-ref-azure-integration-test-commands.shci-operator/step-registry/trusted-execution-clusters/ref/azure-integration-test/trusted-execution-clusters-ref-azure-integration-test-ref.metadata.jsonci-operator/step-registry/trusted-execution-clusters/ref/azure-integration-test/trusted-execution-clusters-ref-azure-integration-test-ref.yaml
|
|
||
| set -euo pipefail | ||
|
|
||
| curl https://sh.rustup.rs | sh -s -- -y |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -u
# Locate and inspect the target command script and nearby context.
target="ci-operator/step-registry/${PWD##*/}/trusted-execution-clusters/ref/azure-integration-test/trusted-execution-clusters-ref-azure-integration-test-commands.sh"
if [ -f "$target" ]; then
wc -l "$target"
sed -n '1,80p' "$target" | cat -n
else
echo "Target not found: $target"
fi
echo '--- matching command scripts ---'
fd -a 'trusted-execution-clusters-ref-azure-integration-test-commands.sh' . || true
echo '--- rustup/azure credential usage ---'
rg -n 'curl .*rustup|rustup|azure|AZURE|[A-Z_]*SECRET|/tmp/secrets|ssh-agent|SHARED_DIR' ci-operator/step-registry 2>/dev/null | head -200Repository: openshift/release
Length of output: 36470
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate and inspect the target command script and nearby context.
target="ci-operator/step-registry/trusted-execution-clusters/ref/azure-integration-test/trusted-execution-clusters-ref-azure-integration-test-commands.sh"
if [ -f "$target" ]; then
echo "== file stats =="
wc -l "$target"
echo "== target content =="
sed -n '1,120p' "$target" | cat -n
else
echo "Target not found: $target"
fi
echo '--- matching command scripts ---'
fd -a 'trusted-execution-clusters-ref-azure-integration-test-commands.sh' . || true
echo '--- rustup/azure credential usage ---'
rg -n 'curl .*rustup|rustup|azure|AZURE|[A-Z_]*SECRET|/tmp/secrets|ssh-agent|SHARED_DIR' ci-operator/step-registry 2>/dev/null | head -200Repository: openshift/release
Length of output: 35715
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate and inspect the target command script and nearby context.
target="ci-operator/step-registry/trusted-execution-clusters/ref/azure-integration-test/trusted-execution-clusters-ref-azure-integration-test-commands.sh"
if [ -f "$target" ]; then
echo "== file stats =="
wc -l "$target"
echo "== target content =="
sed -n '1,120p' "$target" | cat -n
else
echo "Target not found: $target"
echo "candidate files:"
fd -a 'trusted-execution-clusters-ref-azure-integration-test-commands.sh' . || true
fi
echo '--- rustup/azure credential usage ---'
rg -n 'curl .*rustup|rustup|azure|AZURE|[A-Z_]*SECRET|/tmp/secrets|ssh-agent|SHARED_DIR' ci-operator/step-registry 2>/dev/null | head -200Repository: openshift/release
Length of output: 37951
Do not pipe an unverified remote script into the shell.
Line 5 executes mutable remote content directly before Azure credentials are loaded, and the script runs with access to /tmp/secrets/azure. Use a pinned/prebuilt Rust toolchain instead, or download the installer and verify a reviewed checksum/signature before executing the local file.
🤖 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
`@ci-operator/step-registry/trusted-execution-clusters/ref/azure-integration-test/trusted-execution-clusters-ref-azure-integration-test-commands.sh`
at line 5, Replace the direct curl-to-shell invocation with a pinned or prebuilt
Rust toolchain in the trusted execution setup. If retaining the installer,
download it to a local file, verify its reviewed checksum or signature, and only
then execute it; ensure no unverified remote content runs with access to Azure
secrets.
Source: Linters/SAST tools
| git remote add test https://github.com/Jakob-Naucke/trusted-cluster-operator | ||
| git fetch test | ||
| git switch timed-client-ci |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔴 Critical | 🏗️ Heavy lift
Do not run a mutable personal fork with CI cloud credentials.
Lines 28-30 replace the PR checkout with Jakob-Naucke/trusted-cluster-operator:timed-client-ci. Azure authentication has already occurred on Lines 21-24, so make attestation-tests executes unreviewed fork code with cloud authentication. Lines 32-33 also select a mutable personal Quay image instead of a CI-built immutable artifact. Remove these overrides for normal CI; keep debugging branches/images isolated behind an explicit manual path and pin any required revision by commit/digest.
Also applies to: 32-33
🤖 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
`@ci-operator/step-registry/trusted-execution-clusters/ref/azure-integration-test/trusted-execution-clusters-ref-azure-integration-test-commands.sh`
around lines 28 - 30, Remove the `git remote add test`, `git fetch test`, and
`git switch timed-client-ci` overrides so normal CI runs the reviewed checkout;
also remove the mutable personal Quay image override at the corresponding
image-selection symbols. If debugging support is required, isolate it behind an
explicit manual path and pin the source revision by commit and image by digest.
| echo "[INFO] Running attestation tests until failure..." | ||
| while make attestation-tests | ||
| do : | ||
| done |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Make failures fail the step and bound the loop.
With make in the while condition, set -e does not propagate its failure. After any successful iteration, the final : makes the loop return zero when make eventually fails; if tests always pass, the job runs indefinitely. Remove the loop for normal CI or use a bounded retry loop so failures exit non-zero.
🤖 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
`@ci-operator/step-registry/trusted-execution-clusters/ref/azure-integration-test/trusted-execution-clusters-ref-azure-integration-test-commands.sh`
around lines 40 - 43, Update the attestation test execution around the make
attestation-tests loop to avoid unbounded retries and ensure a final test
failure makes the step exit non-zero. Prefer running make attestation-tests once
for normal CI, or implement a bounded retry with an explicit failure exit after
the retry limit.
Following up on openshift#79393. Debug first. Signed-off-by: Jakob Naucke <jnaucke@redhat.com>
0f6b409 to
13cb31e
Compare
|
Infinite loop was successful until timeout. Promising. Let's give it another go including cluster-level tests, which I forgot earlier. /pj-rehearse |
|
@Jakob-Naucke: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
[REHEARSALNOTIFIER]
Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
|
@Jakob-Naucke: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
adding up, 4 good runs in a row (failure is from timeout) |
Following up on #79393. Debug first.
Summary by CodeRabbit
trusted-execution-clustersCI/operator configuration for themainbranch, including a basic builder image stream tag and default CPU requests.azure-test-image, based on the OCP 4.21 CLI image, by copying in theocbinary and installing tooling needed by the tests.trusted-execution-clusters-ref-azure-integration-test) wired to an AWS IPI-based cluster profile (ipi-aws), with Azure credentials mounted from/tmp/secrets/azure.make integration-testsin a loop until it fails.OWNERSfile and a corresponding ref metadata JSON for the new Azure integration-test reference.