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

Allow importing licenses with a missing "usage" attribute #14326

Merged
merged 1 commit into from
Aug 9, 2023

Conversation

phess
Copy link
Contributor

@phess phess commented Aug 8, 2023

SUMMARY

This trivial change fixes AAP-14614.
The original issue happens when AAP is pulling a subscription from Red Hat Satellite, and the subscription is missing a "usage" attribute.
Many subscriptions lack a "usage" attribute, so this shouldn't cause AAP to fail with an error 400.

The error on AAP is a KeyError thrown by sub['usage'].
The fix here is to allow this lookup to return a blank value with sub.get('usage') so the KeyError is never raised and the subscription import occurs just fine.

ISSUE TYPE
  • Bug, Docs Fix or other nominal change
COMPONENT NAME
  • Other
AWX VERSION

No make installed in this lab AAP, so here is the RPM version.

automation-controller-venv-tower-4.4.1-1.el9ap.x86_64
ADDITIONAL INFORMATION

Logged in tower.log before this fix:

2023-08-08 18:32:14,015 ERROR    [db1442fb0fc440bdba589c6a90bd10f5] awx.api.views.root Invalid subscription submitted.
Traceback (most recent call last):
  File "/var/lib/awx/venv/awx/lib64/python3.9/site-packages/awx/api/views/root.py", line 194, in post
    validated = get_licenser().validate_rh(user, pw)
  File "/var/lib/awx/venv/awx/lib64/python3.9/site-packages/awx/main/utils/licensing.py", line 231, in validate_rh
    json = self.get_satellite_subs(host, user, pw)
  File "/var/lib/awx/venv/awx/lib64/python3.9/site-packages/awx/main/utils/licensing.py", line 290, in get_satellite_subs
    license['usage'] = sub['usage']
KeyError: 'usage'
2023-08-08 18:32:14,016 WARNING  [db1442fb0fc440bdba589c6a90bd10f5] awx.api.generics status 400 received by user admin attempting to access /api/v2/config/subscriptions/ from 10.2.69.13
2023-08-08 18:32:14,023 WARNING  [db1442fb0fc440bdba589c6a90bd10f5] django.request Bad Request: /api/v2/config/subscriptions/

Logged after the fix:
Nothing logged at INFO level or worse.

@shanemcd
Copy link
Member

shanemcd commented Aug 8, 2023

Hi @phess - please update the PR title and commit message to follow the guidelines outlined here: https://github.com/ansible/awx/blob/devel/CONTRIBUTING.md#submitting-pull-requests

Specifically, this link: How to write a Git commit message.

@phess phess changed the title Fixes AAP-14614 Allow importing licenses with a missing "usage" attribute Aug 9, 2023
Copy link
Member

@rooftopcellist rooftopcellist left a comment

Choose a reason for hiding this comment

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

Agreed, we should merge this. For extra context, here is an example of a subscription with Usage empty:

Subscription Name:   Red Hat Ansible Automation Platform, Standard (100 Managed Nodes)
Provides:            Red Hat Ansible Automation Platform
                     Red Hat Ansible Engine
                     Red Hat Single Sign-On
                     JBoss Enterprise Application Platform
SKU:                 MCT3691
Contract:            12856738
Pool ID:             8a85f99c7d76f2fd017d96c9f1e606bb
Provides Management: No
Available:           431
Suggested:           1
Service Type:        L1-L3
Roles:               
Service Level:       Standard
Usage:     

@TheRealHaoLiu TheRealHaoLiu merged commit c2d0284 into ansible:devel Aug 9, 2023
14 checks passed
kialam pushed a commit to kialam/awx that referenced this pull request Aug 23, 2023
kialam pushed a commit to kialam/awx that referenced this pull request Aug 23, 2023
djyasin pushed a commit to djyasin/awx that referenced this pull request Sep 16, 2024
djyasin pushed a commit to djyasin/awx that referenced this pull request Nov 11, 2024
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.

5 participants