Repository navigation
CherryPicked: [v2.11] feat(copyoffload): add storage_secret_extra for array-specific Secret keys - #540
Merged
myakove merged 2 commits intoJun 9, 2026
Conversation
… keys (#535) * feat(copyoffload): add storage_secret_extra for array-specific Secret keys Allow extra Forklift stringData keys via providers JSON and COPYOFFLOAD_STORAGE_SECRET_EXTRA env override. Includes generate wizard prompts and copy-offload guide documentation. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(copyoffload): address PR review on storage_secret_extra Add missing docstring sections, use TypeError for type validation, mask wizard secret extra values in prompts and config summary. Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Cursor <cursoragent@cursor.com>
|
Report bugs in Issues Welcome! 🎉This pull request will be automatically processed with the following features:
🔄 Automatic Actions
📋 Available CommandsPR Status Management
Review & Approval
Testing & Validation
Container Operations
Cherry-pick Operations
Label Management
✅ Merge RequirementsThis PR will be automatically approved when the following conditions are met:
📊 Review ProcessApprovers and ReviewersApprovers:
Reviewers:
Available Labels
AI Features
💡 Tips
For more information, please refer to the project documentation or contact the maintainers. |
3 tasks done
Collaborator
|
/verified |
Collaborator
Code Review by Qodo
1. Manual edit under docs/
|
Collaborator
|
/lgtm |
myakove
enabled auto-merge (squash)
June 9, 2026 07:39
|
Clean rebase detected — no code changes compared to previous head ( |
redhat-qe-bot2
added
lgtm-myakove
approved-myakove
can-be-merged
and removed
needs-rebase
labels
myakove
deleted the
CherryPicked-feat/copyoffload-storage-secret-extra-j8yZj
branch
June 9, 2026 07:41
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+220
to
+250
| ### Extra storage secret keys (`storage_secret_extra`) | ||
|
|
||
| Use `storage_secret_extra` when the populator needs additional Kubernetes Secret keys that are not | ||
| mapped by the built-in vendor fields (for example Dell PowerMax `POWERMAX_PORT_GROUP_NAME`). | ||
|
|
||
| Keys in `storage_secret_extra` must match Forklift **Secret `stringData` names** (uppercase). Values are | ||
| merged into the storage secret the test suite creates for copy-offload. They override vendor-mapped keys | ||
| when the same Secret key is specified. | ||
|
|
||
| Use quoted strings for values (for example `"true"`). If you use JSON booleans in | ||
| `COPYOFFLOAD_STORAGE_SECRET_EXTRA` or in `.providers.json`, they are normalized to lowercase `"true"` / | ||
| `"false"` for Secret `stringData`. An empty object or `null` for `storage_secret_extra` is allowed; env-only | ||
| configuration still works. | ||
|
|
||
| **Example (PowerMax):** | ||
|
|
||
| ```json | ||
| "storage_secret_extra": { | ||
| "POWERMAX_PORT_GROUP_NAME": "mtv_group_pg", | ||
| "STORAGE_SKIP_SSL_VERIFICATION": "true" | ||
| } | ||
| ``` | ||
|
|
||
| **Environment override** (takes precedence over `.providers.json`): | ||
|
|
||
| ```bash | ||
| export COPYOFFLOAD_STORAGE_SECRET_EXTRA='{"POWERMAX_PORT_GROUP_NAME":"mtv_group_pg","STORAGE_SKIP_SSL_VERIFICATION":"true"}' | ||
| ``` | ||
|
|
||
| The `mtv-api-tests generate` wizard can prompt for these keys interactively after vendor-specific fields. | ||
|
|
There was a problem hiding this comment.
1. Manual edit under docs/ 📘 Rule violation ⚙ Maintainability
The PR directly modifies a file under docs/, which is disallowed because docs must remain generator-owned to avoid drift. This can cause the generated documentation output to diverge from the source-of-truth.
Agent Prompt
## Issue description
This PR manually edits a file under `docs/`, which is prohibited by compliance policy. Documentation under `docs/` must only change via the documentation generator/source-of-truth process.
## Issue Context
The PR adds a new section about `storage_secret_extra` directly into `docs/copyoffload/how-to-run-copyoffload-tests.md`.
## Fix Focus Areas
- docs/copyoffload/how-to-run-copyoffload-tests.md[220-250]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+60
to
+99
| def _storage_secret_extra_value_as_string(value: Any) -> str: | ||
| """Convert a config/env value to Kubernetes Secret stringData text. | ||
|
|
||
| JSON booleans must become lowercase ``true``/``false``, not Python ``True``/``False``. | ||
|
|
||
| Args: | ||
| value: Raw value from JSON config or environment. | ||
|
|
||
| Returns: | ||
| str: Normalized Secret ``stringData`` text. | ||
| """ | ||
| if isinstance(value, bool): | ||
| return "true" if value else "false" | ||
| return str(value).strip() | ||
|
|
||
|
|
||
| def _secret_extra_entries_from_mapping(mapping: dict[Any, Any]) -> dict[str, str]: | ||
| """Normalize a mapping to non-empty Secret stringData key/value pairs. | ||
|
|
||
| Args: | ||
| mapping: Raw key/value mapping from JSON config or environment. | ||
|
|
||
| Returns: | ||
| dict[str, str]: Normalized Secret keys and values. | ||
|
|
||
| Raises: | ||
| ValueError: If any key is empty after stripping. | ||
| """ | ||
| result: dict[str, str] = {} | ||
| for secret_key, value in mapping.items(): | ||
| if value is None: | ||
| continue | ||
| text = _storage_secret_extra_value_as_string(value) | ||
| if not text: | ||
| continue | ||
| key = str(secret_key).strip() | ||
| if not key: | ||
| raise ValueError("storage_secret_extra keys must be non-empty strings") | ||
| result[key] = text | ||
| return result |
There was a problem hiding this comment.
2. Extra values get altered 🐞 Bug ≡ Correctness
_storage_secret_extra_value_as_string() uses str(value).strip(), which removes leading/trailing whitespace/newlines from extra Secret values and can corrupt whitespace-sensitive data (e.g., PEM/cert blobs or tokens) before writing to the Kubernetes Secret. It also coerces non-scalar JSON values (lists/objects) into Python string forms (e.g., dicts become strings with single quotes), silently producing unexpected Secret contents rather than failing fast.
Agent Prompt
### Issue description
`storage_secret_extra` values are normalized via `str(value).strip()`, which can modify intended values by trimming whitespace/newlines. Additionally, complex JSON types (objects/arrays) are stringified implicitly, yielding surprising non-JSON text.
### Issue Context
These values are merged into the `Secret.stringData` used by copy-offload, so any normalization issues become real cluster Secret data.
### Fix Focus Areas
- utilities/copyoffload_migration.py[60-99]
### Suggested fix
- Change `_storage_secret_extra_value_as_string` so that:
- `bool` -> "true"/"false" (keep)
- `str` -> return as-is (do not strip)
- `int`/`float` -> `str(value)` (no strip)
- `dict`/`list`/other complex types -> raise `TypeError` with a clear message like: "storage_secret_extra values must be strings (or JSON booleans)"
- In `_secret_extra_entries_from_mapping`, decide emptiness without mutating values, e.g. skip only when `value is None` or (isinstance(value, str) and value.strip() == "").
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+76
to
+99
| def _secret_extra_entries_from_mapping(mapping: dict[Any, Any]) -> dict[str, str]: | ||
| """Normalize a mapping to non-empty Secret stringData key/value pairs. | ||
|
|
||
| Args: | ||
| mapping: Raw key/value mapping from JSON config or environment. | ||
|
|
||
| Returns: | ||
| dict[str, str]: Normalized Secret keys and values. | ||
|
|
||
| Raises: | ||
| ValueError: If any key is empty after stripping. | ||
| """ | ||
| result: dict[str, str] = {} | ||
| for secret_key, value in mapping.items(): | ||
| if value is None: | ||
| continue | ||
| text = _storage_secret_extra_value_as_string(value) | ||
| if not text: | ||
| continue | ||
| key = str(secret_key).strip() | ||
| if not key: | ||
| raise ValueError("storage_secret_extra keys must be non-empty strings") | ||
| result[key] = text | ||
| return result |
There was a problem hiding this comment.
3. Extra key casing unvalidated 🐞 Bug ⚙ Maintainability
Docs require storage_secret_extra keys to match Forklift Secret stringData names (uppercase), but _secret_extra_entries_from_mapping() only strips keys and accepts any casing. Mis-cased keys will be written under the wrong name (Kubernetes Secret keys are case-sensitive), so the expected populator key will be absent and failures will surface later during copy-offload execution.
Agent Prompt
### Issue description
`storage_secret_extra` keys are documented as uppercase Forklift Secret `stringData` names, but the parser accepts any casing without validation. This makes configuration mistakes easy and failures delayed.
### Issue Context
The docs and example config explicitly state the uppercase requirement.
### Fix Focus Areas
- utilities/copyoffload_migration.py[76-99]
- docs/copyoffload/how-to-run-copyoffload-tests.md[220-227]
- .providers.json.example[89-95]
### Suggested fix
- Enforce the documented contract in `_secret_extra_entries_from_mapping`:
- After `key = str(secret_key).strip()`, add a check like `if key.upper() != key: raise ValueError("storage_secret_extra keys must be uppercase Secret stringData names")`.
- Alternatively, normalize with `key = key.upper()` and log a warning that the key was normalized (but raising is stricter and matches the docs).
- Keep the existing non-empty key check.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
|
New container for ghcr.io/redhatqe/mtv-api-tests:v2.11 published |
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
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.
Cherry-pick from
mainbranch, original PR: #535, PR owner: prabinovRedhat