feat: add PR-scoped TA Validator exceptions - #539
mkolasinski-splunk wants to merge 39 commits into
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 84f5cf6264
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
84f5cf6 to
3e77213
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 04055f2542
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
bd21624 to
824a474
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 824a474a68
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cd7857318a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9b2c1bc2d1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| uses: actions/upload-artifact@v7 | ||
| with: | ||
| name: ta-validator-pr-exceptions |
There was a problem hiding this comment.
When validation initially succeeds and a user follows the README by editing the exception comment and selecting Re-run all jobs, this step uploads ta-validator-pr-exceptions again within the same workflow run. Artifacts are immutable, and the upload-artifact documentation requires overwrite: true to replace an existing name, so preparation fails before run-gs-scorecard can evaluate the edited exceptions. Fresh evidence beyond the prior rerun comment is this newly added fixed-name upload without overwrite handling; enable overwrite or use a unique attempt-specific artifact name that the download step also selects.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 85125abb07
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| * `python-version` - Python version used for testing, default `3.9` | ||
| * `spl2-generate` - when `true` enables SPL2 generation, default `false` | ||
| * `gs-image-version` - version of the GS Scorecard Docker image, default `1.2` | ||
| * `gs-image-version` - version of the TA Validator Docker image, default `mr-140-0431e2581e5d-amd64` while the paired PR-exception interface is awaiting an official release |
There was a problem hiding this comment.
This input reference still names mr-140-0431e2581e5d-amd64, while the workflow default at .github/workflows/reusable-build-test-release.yml:74 and the troubleshooting section at README.md:708 use mr-140-3fee426db3a8-amd64 for the new effective-file interface. A caller following this list and explicitly setting the documented value will override the compatible default with the previous paired-interface image, potentially causing the new merge/effective-file flow to fail.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7b36368d4a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| exit 1 | ||
| fi | ||
| if [ -f "$effective_exceptions_path" ]; then | ||
| cp "$effective_exceptions_path" "$GITHUB_WORKSPACE/.ta-validator-exceptions.yaml" |
There was a problem hiding this comment.
On pull requests, this destination is controlled by the checked-out branch, so a committed .ta-validator-exceptions.yaml symlink causes cp to follow the link and overwrite its target rather than installing a regular effective file. This permits a PR to corrupt another writable runner file, such as .git/config, before the credentialed validator runs. GNU cp --help documents --remove-destination as removing the destination before opening it; use that option or explicitly reject/remove a destination symlink first.
Useful? React with 👍 / 👎.
mkolasinski-splunk
changed the title
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 77beb8f3b8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| run-gs-scorecard: | ||
| name: quality-gs-scorecard | ||
| prepare-ta-validator-exceptions: | ||
| if: ${{ github.event_name == 'pull_request' }} |
There was a problem hiding this comment.
For docs-only PRs or PRs without execute_gs_scorecard, setup-workflow leaves the scorecard disabled and run-gs-scorecard is skipped, but this job still requests an app token and parses or creates the exception comment. If that unused operation fails—for example because an existing comment is malformed or the app/API is unavailable—the job fails, and pre-publish includes its result without excluding it, so an unselected informational check can make the workflow fail. Gate preparation on execute-gs-scorecard, or ignore its result when that output is false.
Useful? React with 👍 / 👎.
| aws-region: us-west-2 | ||
| - name: Upload GS Scorecard report | ||
| uses: actions/upload-artifact@v7 | ||
| uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a |
There was a problem hiding this comment.
sha references were suggested by agentic readiness skill - let me double check if it's really required.
There was a problem hiding this comment.
| - `SPL_COM_USER` and `SPL_COM_PASSWORD` for AppInspect integration | ||
|
|
||
| - Check that the Docker image version specified via the `gs-image-version` workflow input (`GS_IMAGE_VERSION` env var, default `1.2`) exists in the ECR registry. The GS Scorecard tool version is controlled separately via `gs-version` input (`GS_VERSION` env var, default `0.3`). | ||
| - Check that the Docker image version specified via the `gs-image-version` workflow input (`GS_IMAGE_VERSION` env var, default `mr-140-3fee426db3a8-amd64`) exists in the ECR registry. This immutable pre-release image implements the effective-file interface; replace it with the compatible official image after the TA Validator release. The TA Validator tool version is controlled separately via `gs-version` input (`GS_VERSION` env var, default `0.3`). |
There was a problem hiding this comment.
Shouldn't we wait with this PR until MR140 is merged?
There was a problem hiding this comment.
Yes, indeed. First TA Validator needs to be released, then this reference needs to be updated.
3 participants
Implements the reusable-workflow side of PR-scoped TA Validator exceptions.
Permanent and temporary exceptions use the same canonical YAML schema. The workflow extracts only the marked YAML from the automation-managed PR comment; the TA Validator image validates and merges it with the repository's
.ta-validator-exceptions.yaml. Evaluation then consumes the single effective file through the normal TA Validator path.Interface
image,mode, and AWS authentication inputs.mergereads$RUNNER_TEMP/ta-validator-comment-exceptions.yamland writes$RUNNER_TEMP/.ta-validator-exceptions.yaml.ta-validator-exceptionsartifact.evaluatecopies that file to the add-on's conventional.ta-validator-exceptions.yamlpath when present, then runs normal evaluation.Related implementation
Revisions and image
44b3866bfe43de043f93f8d9e317616abf3e52c82c974a3e03ac35c12832742e5e689729001b87713fee426db3a8e6a6031a4990fec032f83661ee9b956110764581.dkr.ecr.us-west-2.amazonaws.com/ta-automation/gs-scorecard:mr-140-3fee426db3a8-amd64sha256:6208da305520a85a3f067ff26caa66eb91371be3c1a53120d77591db2c1d601cReview checks
44b3866bfe43de043f93f8d9e317616abf3e52c8: pre-commit, copyright compliance, and publish passed.2c974a3e03ac35c12832742e5e689729001b8771:pre_commitandunit_testspassed.AWS TA end-to-end scenarios
These behavioral runs predate the review follow-up above: they used reusable-workflow revision
77beb8f3b8801e3483f668d773114f08e3a91deband runtime revision3fee426db3a8e6a6031a4990fec032f83661ee9b, with no caller image override. Logs confirm the default image resolved to digestsha256:6208da305520a85a3f067ff26caa66eb91371be3c1a53120d77591db2c1d601c. The follow-up changes only replace third-party action commit pins with version tags and address TA Validator input compatibility/report rendering; their exact heads are covered by the review checks above.bc55bdeaedb59178fbccdda0d08f5dcedce4e0ae: run 35873298969, merge job passed, effective file, TA Validator job passed, and the report contains the expected check-wide XFAIL.3bbd4f59c922037effc7f276cb451dae31e83d3d: run 35873127028, merge job passed, effective file, TA Validator job passed, and the report contains both applied declarations.7ed1c72decd3aa3e3cbb43b559345efd6e167041: run 35873133659, merge job passed, effective file, TA Validator job passed, and the report contains both applied declarations.da1d95c6faa1e1bf3c7d2080204f1eb8470c08af: run 35873139936; the merge job failed fast as expected, and TA Validator evaluation was skipped.1343a9659778b79520e9934112a29c788d147baa: run 35873149066; the merge job failed fast as expected, and TA Validator evaluation was skipped.The three positive runs have an overall
failureconclusion because unrelated PR-title, functional-test-presence, and FOSSA vulnerability jobs failed; their exception preparation and TA Validator jobs succeeded. The two negative runs failed by design at exception preparation. The effective files preserve permanent declarations before temporary declarations, and the three positive reports expose onlycategoryandreasonin exception metadata. All five test PRs are closed with their branches and workflow history preserved.Local verification: 13 workflow contract tests and all repository pre-commit hooks passed.