content
content copied to clipboard
Remove jinja condition to make rule applicability to all products in Kerberos rules
Description:
- Remove jinja condition to make rule applicability to all products in Kerberos rules.
Rationale:
- The applicability should be there for all products, not only part of them. For example RHEL9 and OL9 should have a more recent version of the package so it should return not applicable.
Start a new ephemeral environment with changes proposed in this pull request:
@Xeicker FYI, I'm removing the jinja conditions here since it should be applicable to all products and the version of the package will determine the applicability...
The changes LGTM.
But as distros adopt newer versions of
krb5this rule will not be needed anymore. So how about also restricting the products this rule is available for?We should avoid removing the rule from a product that already released with this rule. But explicitly listing the current prodtypes, should avoid automatically adding these rules to new products, or product versions. And then, in 10 years, when all current products are EOL, these rules can be deleted, :)
a comment on the prodtype can help preventing people from adding new products in the future, but it's not bulletproof. Some kind of negate mechanism would be the best, like: !rhel10
The changes LGTM. But as distros adopt newer versions of
krb5this rule will not be needed anymore. So how about also restricting the products this rule is available for? We should avoid removing the rule from a product that already released with this rule. But explicitly listing the current prodtypes, should avoid automatically adding these rules to new products, or product versions. And then, in 10 years, when all current products are EOL, these rules can be deleted, :)a comment on the
prodtypecan help preventing people from adding new products in the future, but it's not bulletproof. Some kind of negate mechanism would be the best, like:!rhel10
I would prefer to work with "allow list" instead "deny list". We already know the current relevant products, but we don't know about future products. Also, working with "deny list", technically we should increase the list whenever a new product appears.I believe this is avoidable maintenance. In any case, the comment on prodtype is handy.
I have refreshed the PR with some more changes and I believe it should be good to go.
There was a problem in the CPE conditionals which hopefully is fixed by: bbd391e
Start 186: rhel9-bash-shellcheck
185/262 Test #186: rhel9-bash-shellcheck .................................................***Failed 19.60 sec
In /__w/content/content/build/rhel9/fixes/bash/kerberos_disable_no_keytab.sh line 3:
if [ ! -f /.dockerenv ] && [ ! -f /run/.containerenv ] && { ( ); }; then
^-- SC1073: Couldn't parse this explicit subshell. Fix to allow more checks.
^-- SC1072: Unexpected keyword/token. Fix any mentioned problems and try again.
In /__w/content/content/build/rhel9/fixes/bash/package_krb5-workstation_removed.sh line 7:
if ( ); then
^-- SC1073: Couldn't parse this explicit subshell. Fix to allow more checks.
^-- SC1072: Unexpected keyword/token. Fix any mentioned problems and try again.
For more information:
https://www.shellcheck.net/wiki/SC1072 -- Unexpected keyword/token. Fix any...
https://www.shellcheck.net/wiki/SC1073 -- Couldn't parse this explicit subs...
Code Climate has analyzed commit 5aa839d0 and detected 3 issues on this pull request.
Here's the issue category breakdown:
| Category | Count |
|---|---|
| Duplication | 2 |
| Complexity | 1 |
The test coverage on the diff in this pull request is 100.0% (50% is the threshold).
This pull request will bring the total coverage in the repository to 41.2% (0.4% change).
View more on Code Climate.
The Code Climate issues can be waived - the 2 sections in code marked as duplicate actually works with different type and unifying them would not improve the readability; and changing CPEALLogicalTest is out of scope of this PR.
@ggbecker: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:
| Test name | Commit | Details | Required | Rerun command |
|---|---|---|---|---|
| ci/prow/e2e-aws-ocp4-high-node | 5aa839d047f1e0eb304b652932fd41ff0ec5999d | link | true | /test e2e-aws-ocp4-high-node |
Full PR test history. Your PR dashboard.
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.