test: add acceptance coverage for the ITS pipeline - #3591
dheerajodha wants to merge 3 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (2)
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review. 📝 WalkthroughWalkthroughThe acceptance framework now runs repository pipeline definitions in kind and checks PipelineRun outcomes and results. New scenarios cover trusted signed images, missing images, and untrusted signatures. The README documents the coverage, and a task-test ConfigMap name changed. ChangesITS pipeline acceptance
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Other Sequence Diagram(s)sequenceDiagram
participant GodogSteps
participant kindCluster
participant KubernetesAPI
GodogSteps->>kindCluster: RunPipeline with pipeline parameters
kindCluster->>KubernetesAPI: Create PipelineRun
GodogSteps->>kindCluster: AwaitUntilPipelineIsDone
kindCluster->>KubernetesAPI: Poll PipelineRun
KubernetesAPI-->>kindCluster: Completion status and results
kindCluster-->>GodogSteps: PipelineInfo
Suggested reviewers: Merge Risk: ⚪ Minimal · up to The added kind scenarios check trusted-image success and strict-dependent failure handling, including the pipeline result. Their assertions match the inspected pipeline behavior; no concrete merge-blocking risk remains. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 6 files. (2 skipped: 2 unsupported.)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Comment |
|
Risk Assessment: moderate (2/5) DetailsAll changes are test/acceptance code with no production paths touched, but the XL size, new Cluster-interface method additions, and rename of a shared test fixture in a stable directory each contribute incremental integration risk, landing the PR at moderate. |
|
Looks good to me |
![fullsend-ai-review[bot]](https://avatars.githubusercontent.com/in/3478501?s=60&v=4){kind=small}
fullsend-ai-review
Bot
added
the
requires-manual-review
Codecov Report✅ All modified and coverable lines are covered by tests.
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
|
🤖 Finished Review · ❌ Failure (ensuring provider "vertex-ai": provider create "vertex-ai" failed: exit status 1 (output: Error: × code: 'Client specified an invalid argument', message: "provider │ credentials are not declared by pr…) · Started 1:12 PM UTC · Completed 1:12 PM UTC Commit: Effort: high |
|
🤖 Finished Review · ❌ Failure (ensuring provider "vertex-ai": provider create "vertex-ai" failed: exit status 1 (output: Error: × code: 'Client specified an invalid argument', message: "provider │ credentials are not declared by pr…) · Started 11:52 AM UTC · Completed 11:52 AM UTC Commit: Effort: high |
st3penta
left a comment
st3penta
left a comment
There was a problem hiding this comment.
lgtm, with a couple of nitpicks
| @@ -0,0 +1,41 @@ | |||
| @its-pipeline | |||
There was a problem hiding this comment.
Looks like it, idky the bot thought we'd need a GoDog tag for focused testing of these scenarios in main. Removed, thanks!
| Scenario Outline: ITS pipeline handles validation failures according to STRICT | ||
| When version 0.1 of the pipeline named "enterprise-contract" is run with parameters: | ||
| | SNAPSHOT | {"components": [{"containerImage": "${REGISTRY}/acceptance/its-missing"}]} | |
There was a problem hiding this comment.
nitpick: this scenario doesn't create the image 'its-missing', so it tests image lookup failure rather than a validation failure. Consider adding something like:
Given an image named "acceptance/its-missing"
And a valid attestation of "acceptance/its-missing" signed by the "known" key
so that the validation fails for the missing image signature.
The outcome of the test is the same, but the scenario is more precise
There was a problem hiding this comment.
Good point, I modified that into image lookup failures scenario, and added 2 new scenarios for "ITS pipeline handles untrusted signatures".
|
🤖 Finished Review · ❌ Failure (ensuring provider "github-ro": provider create "github-ro" failed: exit status 1 (output: Error: × code: 'Client specified an invalid argument', message: "provider │ credentials are not declared by pr…) · Started 1:23 PM UTC · Completed 1:23 PM UTC Commit: Effort: high |
|
🤖 Finished Review · ❌ Failure (ensuring provider "vertex-ai": provider create "vertex-ai" failed: exit status 1 (output: Error: × code: 'Client specified an invalid argument', message: "provider │ credentials are not declared by pr…) · Started 1:29 PM UTC · Completed 1:29 PM UTC Commit: Effort: high |
This branch has not been deployed
2 participants
What:
Add acceptance coverage for the ITS pipeline in a kind cluster using the pipeline definition and task bundles from the checkout. Five scenario executions cover a trusted signed image passing validation, image lookup failures with both values of STRICT, and untrusted image/attestation signatures with both values of STRICT. Each checks both PipelineRun completion and the pipeline's TEST_OUTPUT result.
The untrusted-signature scenarios create and sign an image with an untrusted key, then verify it against the known public key. The missing-image scenarios remain separate lookup-failure coverage.
The Kubernetes test helpers launch and await PipelineRuns with a bounded timeout. Rename the existing keyless ConfigMap fixture so its dummy service URLs do not affect concurrent pipeline tests.
Why:
Catch pipeline wiring and verification regressions in the CLI repository before merge, complementing the separate e2e coverage. The scenarios run in the existing acceptance suite and PR Checks workflow, which already includes pipeline-only changes.
Validation:
CGO_ENABLED=0 go test ./kubernetes/...in the acceptance module passed (package compilation), including after rebasing. No Go helpers changed in the scenario expansion../kubernetes/...previously passed with zero issues.git diff --check: passed.Tickets:
EC-1948