-
Notifications
You must be signed in to change notification settings - Fork 43
certsuite | update backwards compatibility till v5.5.0 #693
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
📝 WalkthroughWalkthroughThis update raises the minimum supported Certsuite version to v5.5.0, removes version-specific logic and variables, and streamlines related documentation and configuration. Version checks and conditional code paths are simplified, making certain options and flags apply universally rather than being gated by specific Certsuite versions. Changes
Possibly related PRs
Suggested reviewers
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
|
Build succeeded. ✔️ dci-rpm-build-el8 SUCCESS in 2m 52s |
fb5b60f to
05818bb
Compare
|
Build succeeded. ✔️ dci-rpm-build-el8 SUCCESS in 2m 55s |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
roles/k8s_best_practices_certsuite/tasks/tests.yml (1)
2-13:⚠️ Potential issueRestore directory creation for config and result mounts
The tasks that created{{ kbpc_certsuite_dir }}/config_filesandresult_fileswere removed, but the subsequentpodman runmounts will fail if those host paths don’t exist. Please reintroduce file tasks to ensure these directories are present before mounting.
🧹 Nitpick comments (2)
roles/k8s_best_practices_certsuite/README.md (2)
12-12: Use "until" instead of "till" in the description
For a more formal tone, replace "till" with "until" in thekbpc_versiondescription.🧰 Tools
🪛 LanguageTool
[style] ~12-~12: Consider using the more formal “until”.
Context: ... version. The role supports from v5.5.0 till the latest stable version. HEAD version...(TILL)
[uncategorized] ~12-~12: These words/punctuation marks might seem a little out of order. For clarity and coherence, try switching them around.
Context: .... HEAD version (in the main branch) can be also used, but it is not guaranteed a comple...(AI_EN_LECTOR_REPLACEMENT_WORD_ORDER)
143-143: Add missing article "the" for clarity
Consider updating to "kbpc_cwe_apikey_path: the path to the file containing the CWE Hydra API key..." to improve readability.🧰 Tools
🪛 LanguageTool
[uncategorized] ~143-~143: You might be missing the article “the” here.
Context: ...d to configure these two variables with proper values: -kbpc_cwe_apikey_path: path...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
roles/k8s_best_practices_certsuite/README.md(4 hunks)roles/k8s_best_practices_certsuite/defaults/main.yml(0 hunks)roles/k8s_best_practices_certsuite/tasks/pre-run.yml(1 hunks)roles/k8s_best_practices_certsuite/tasks/tests.yml(1 hunks)roles/k8s_best_practices_certsuite/templates/certsuite_config.yml.j2(0 hunks)
💤 Files with no reviewable changes (2)
- roles/k8s_best_practices_certsuite/defaults/main.yml
- roles/k8s_best_practices_certsuite/templates/certsuite_config.yml.j2
🧰 Additional context used
🪛 LanguageTool
roles/k8s_best_practices_certsuite/README.md
[style] ~12-~12: Consider using the more formal “until”.
Context: ... version. The role supports from v5.5.0 till the latest stable version. HEAD version...
(TILL)
[uncategorized] ~12-~12: These words/punctuation marks might seem a little out of order. For clarity and coherence, try switching them around.
Context: .... HEAD version (in the main branch) can be also used, but it is not guaranteed a comple...
(AI_EN_LECTOR_REPLACEMENT_WORD_ORDER)
[uncategorized] ~143-~143: You might be missing the article “the” here.
Context: ...d to configure these two variables with proper values: - kbpc_cwe_apikey_path: path...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~145-~145: Loose punctuation mark.
Context: ...proper values: - kbpc_cwe_apikey_path: path to CWE Hydra API key, generated be...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~146-~146: Loose punctuation mark.
Context: ...n you create it. - kbpc_cwe_project_id: CNF Certification project ID that is pr...
(UNLIKELY_OPENING_PUNCTUATION)
🪛 markdownlint-cli2 (0.17.2)
roles/k8s_best_practices_certsuite/README.md
145-145: Bare URL used
null
(MD034, no-bare-urls)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Sanity Check (stable-2.9)
- GitHub Check: Sanity Check (stable-2.17)
🔇 Additional comments (2)
roles/k8s_best_practices_certsuite/tasks/pre-run.yml (1)
9-15: Minimum supported Certsuite version updated to 5.5.0
The version check and failure message have been correctly updated to enforce v5.5.0 as the new lower bound.roles/k8s_best_practices_certsuite/tasks/tests.yml (1)
60-63: Include Connect API flags whenever credentials are defined
The version-based gating for--connect-api-keyand--connect-project-idhas been removed in favor of checking variable presence, which aligns with the new minimum version policy.
|
from change #693: |
manurodriguez
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
SUMMARY
As agreed in the last Certification sync, and with the idea of simplifying the certsuite role, updating the oldest supported version to v5.5.0, where the last breaking changes were integrated (i.e. the update of the intrusive label).
ISSUE TYPE
Tests
Test-Hints: no-check