Skip to content
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

Refs #37151 - Fix --force flag to use option_override_name #979

Merged
merged 3 commits into from
Dec 20, 2024

Conversation

jeremylenz
Copy link
Member

@jeremylenz jeremylenz commented Dec 17, 2024

#977 should have validated option_override_name, but inadvertently looked at option_value instead. This caused hammer content-override to break completely.

Turns out our Hammer tests were also wrong. Params should be like

--override-name enabled
--value true

--override-name enabled
--value 1

but instead were like

--value enabled

To test:

hammer activation-key content-override --id 2 --content-label satellite-client-6-for-rhel-9-x86_64-rpms --value true
hammer activation-key content-override --id 2 --content-label satellite-client-6-for-rhel-9-x86_64-rpms --override-name foo --value 0
  1. Without the --override-name flag, values true, false, 0, and 1 (and really, any value) should work.
  2. With the --override-name of enabled, the values above should also work.
  3. With the --override-name of anything other than enabled, you should get the (new & improved) error
Could not update content override:
  You must use --force to set an override other than 'enabled'

@chris1984 chris1984 self-assigned this Dec 19, 2024
Copy link
Member

@chris1984 chris1984 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks great, ACK

Before PR:

[root@ip-10-0-167-154 ~]# hammer activation-key content-override --id 1 --content-label satellite-client-6-for-rhel-9-x8
6_64-rpms --value true
Could not update content override:
  You must use --force to set a value other than 'enabled'
  
  [root@ip-10-0-167-154 ~]# hammer activation-key content-override --id 1 --content-label satellite-client-6-for-rhel-9-x8
6_64-rpms --override-name foo --value 0
Could not update content override:
  You must use --force to set a value other than 'enabled'

After PR:

[root@ip-10-0-167-154 ~]# hammer activation-key content-override --id 1 --content-label satellite-client-6-for-rhel-9-x86_64-rpms --value true
Updated content override.
[root@ip-10-0-167-154 ~]# hammer activation-key content-override --id 1 --content-label satellite-client-6-for-rhel-9-x86_64-rpms --override-name foo --value 0
Could not update content override:
  You must use --force to set an override other than 'enabled'
  [root@ip-10-0-167-154 ~]# hammer activation-key content-override --id 1 --content-label satellite-client-6-for-rhel-9-x86_64-rpms --override-name enabled --value 0
Updated content override.

[root@ip-10-0-167-154 ~]# hammer activation-key content-override --id 1 --content-label satellite-client-6-for-rhel-9-x86_64-rpms --override-name foo --value 0 --force
Updated content override.

@jeremylenz jeremylenz merged commit 49bffbb into Katello:main Dec 20, 2024
10 of 11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants