[hooks] Add Barbican secret-store migrate live test hook - #4147
Draft
vakwetu wants to merge 2 commits into
Draft
Conversation
Tempest cannot prove a secret changed backends. This hook creates a secret on the global default store, migrates it with OSC and barbican-secret-migrate, and checks MariaDB for the expected plugin. Related-Issue: #OSPRH-35280 Assisted-by: Cursor Grok 4.6 <noreply@cursor.com> Signed-off-by: Ade Lee <alee@redhat.com>
Contributor
|
Skipping CI for Draft Pull Request. |
Contributor
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Drop MariaDB/sql checks. Assert placement with openstack secret get (microversion 1.3 fields). Discover stores via openstacksdk. Keep admin CLI project-scoped token and pinned payload. Related-Issue: #OSPRH-35280 Assisted-by: Cursor Grok 4.5 Signed-off-by: Ade Lee <alee@redhat.com>
vakwetu
marked this pull request as ready for review
September 2, 2026 20:34
vakwetu
force-pushed
the
feature/OSPRH-35280-secret-store-migrate
branch
from
September 2, 2026 20:34
6435806 to
5bcadc7
Compare
PR Summary by QodoAdd Barbican secret-store migration live-test hook
AI Description
Diagram
High-Level Assessment
Files changed (5)
|
Code Review by Qodo
1. Scenario blocks omit rescue
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| - _payload_after.stdout | trim == cifmw_barbican_migrate_payload | ||
| fail_msg: >- | ||
| {{ cifmw_barbican_migrate_case }}: payload mismatch after migrate. | ||
| always: |
There was a problem hiding this comment.
1. Scenario blocks omit rescue 📘 Rule violation ◔ Observability
The complex migration scenarios proceed directly from block to always without a rescue path that reports failure state and explicitly terminates the run. Failures—particularly those from no_log tasks—therefore lack the structured diagnostics required by the checklist.
Agent Prompt
## Issue description
The migration scenario blocks have cleanup under `always`, but no `rescue` path that exposes relevant failure context and terminates with `ansible.builtin.fail`.
## Issue Context
Report safe diagnostic fields such as the failed task name, return code, and sanitized stderr. Preserve cleanup in `always` and avoid exposing tokens, payloads, or credentials from `no_log` tasks.
## Fix Focus Areas
- hooks/playbooks/barbican-secret-store-migrate/one.yml[154-177]
- hooks/playbooks/barbican-secret-store-migrate/metaonly.yml[87-115]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| gather_facts: false | ||
| vars: | ||
| cifmw_barbican_migrate_namespace: "{{ namespace | default(cifmw_openstack_namespace) | default('openstack') }}" | ||
| cifmw_barbican_migrate_db_account: barbican |
There was a problem hiding this comment.
2. Static pod name variable 📘 Rule violation ⚙ Maintainability
cifmw_barbican_migrate_osc_pod_name stores the static value openstackclient and is consumed only once by the pod lookup. This adds unnecessary indirection without a demonstrated reuse or configuration benefit.
Agent Prompt
## Issue description
The play introduces a variable solely to hold the single-use static value `openstackclient`.
## Issue Context
Inline the value in the pod lookup. If configurability is required, define an externally overridable default rather than an unconditional play variable and document that configuration purpose.
## Fix Focus Areas
- hooks/playbooks/barbican-secret-store-migrate/migrate.yml[38-54]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+232
to
+233
| cifmw_barbican_migrate_v1: "{{ (_barbican_url | regex_replace('/$', '')) + ('' if _barbican_url.rstrip('/').endswith('/v1') else '/v1') }}" | ||
| vars: |
There was a problem hiding this comment.
3. Wrong default backend accepted 🐞 Bug ≡ Correctness
The store validation only requires SimpleCrypto, PKCS#11, and one global default, but _store_other treats every default other than PKCS#11 as SimpleCrypto. If a third backend is global, secrets migrate from that backend to PKCS#11, so the hook never exercises the required SimpleCrypto-to-PKCS#11 path.
Agent Prompt
## Issue description
The migration hook accepts a third backend as the global default and then incorrectly treats it as the SimpleCrypto source, leaving the intended SimpleCrypto/PKCS#11 path untested.
## Issue Context
Store validation proves that both target backends exist but does not prove that the global default is one of them. Secret creation uses the global default, so the scenario must reject any other default backend.
## Fix Focus Areas
- hooks/playbooks/barbican-secret-store-migrate/migrate.yml[218-233]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
vakwetu
marked this pull request as draft
September 2, 2026 20:38
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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
pre_testshook that live-migrates a Barbican secret between SimpleCrypto and PKCS#11 and asserts the backend in MariaDB.barbican-secret-migrateround-trip, dry-run, and metadata-only 409.hooks/playbooks/barbican-secret-store-migrate/so they do not clutterhooks/playbooks/.Prerequisite: the control plane must enable both stores (
simple_cryptoandpkcs11). Current Luna/Proteccio jobs only enablepkcs11; that is a follow-up in the component job / testproject.Test plan
pre-commit run --all-files(ansible-lint)Related-Issue: #OSPRH-35280
Related: OSPRH-35261, OSPRH-35331, RHOSSTRAT-1435
Assisted-by: Cursor Grok 4.6