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

fix: rh_subscription org value #5390

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions cloudinit/config/cc_rh_subscription.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,14 +47,14 @@
"""\
rh_subscription:
activation-key: foobar
org: 12345
org: 'ABC'
"""
),
dedent(
"""\
rh_subscription:
activation-key: foobar
org: 12345
org: 'ABC'
auto-attach: true
service-level: self-support
add-pool:
Expand Down
4 changes: 2 additions & 2 deletions cloudinit/config/schemas/schema-cloud-config-v1.json
Original file line number Diff line number Diff line change
Expand Up @@ -2478,8 +2478,8 @@
"description": "The activation key to use. Must be used with ``org``. Should not be used with ``username`` or ``password``"
},
"org": {
"type": "integer",
"description": "The organization number to use. Must be used with ``activation-key``. Should not be used with ``username`` or ``password``"
"type": "string",
Copy link
Contributor

@aciba90 aciba90 Jun 11, 2024

Choose a reason for hiding this comment

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

As the value of this is ultimately going to be passed as string to subscription-manager and in order to maintain backwards compatibility, I think we should maintain both types as valid and mark integer as deprecated.

Copy link
Author

Choose a reason for hiding this comment

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

Thank you for the feedback. I will address this in a new commit!

Copy link
Member

Choose a reason for hiding this comment

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

@pneigel-ca any updates on this?

Copy link
Author

Choose a reason for hiding this comment

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

@holmanb No updates. If someone can handle in a new PR, I'm welcome to it. I haven't had the time to familiarize myself with the deprecation syntax and rebase with the changes to documentation.

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

@pneigel-ca Is [email protected] your preferred email for the commit message?

Copy link
Author

Choose a reason for hiding this comment

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

@holmanb Thanks for checking in - I didn't configure my email properly in git 😄

[email protected] can be used.

"description": "The organization name to use. Must be used with ``activation-key``. Should not be used with ``username`` or ``password``"
},
"auto-attach": {
"type": "boolean",
Expand Down
2 changes: 1 addition & 1 deletion tests/unittests/config/test_cc_rh_subscription.py
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ class TestBadInput(CiTestCase):
"rh_subscription": {
"activation-key": "abcdef1234",
"fookey": "bar",
"org": "123",
"org": "ABC",
}
}

Expand Down
1 change: 1 addition & 0 deletions tools/.github-cla-signers
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,7 @@ outscale-mdr
philsphicas
phsm
phunyguy
pneigel-ca
qubidt
r00ta
RedKrieg
Expand Down
Loading