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

Fixes #38170 - Explicitly handle content_view params during AK create #11293

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jeremylenz
Copy link
Member

What are the changes introduced in this pull request?

The find_cve_for_single method in the activation keys controller handles the logic of finding the content view environment when you've passed in separate content_view and lifecycle_environment params. However, before this change it did not handle content_view param correctly; it ignored it. The effect was a false positive - the activation key would get created, but not with the content view you specified.

With this change, you should correctly get the error

Environment ID and content view ID must be provided together

Considerations taken when implementing this change?

What are the testing steps for this pull request?

Test hammer activation-key create
Also test the web UI and make sure nothing breaks there with creating activation keys.

@LadislavVasina1
Copy link

I can confirm that this patch fixes the issue. Tested with upstream packit build.
PRE-PATCH ❌

hammer activation-key create --name="testAK_1" --organization="testORG" --content-view="testCV"
Activation key created.

POST-PATCH ✅

# hammer activation-key create --name="testAK_1" --organization="testORG" --content-view="testCV"
Could not create the activation key:
  Environment ID and content view ID must be provided together

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