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.

Sign up for GitHub

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

Jump to bottom

redhat_subscription: drop unneeded args to Rhsm.register() #5583

Merged
merged 1 commit into from
Nov 29, 2022
Merged

redhat_subscription: drop unneeded args to Rhsm.register() #5583

merged 1 commit into from
Nov 29, 2022

Conversation

ptoscano
Copy link
Contributor

SUMMARY

Stop passing all the rhsm_, and server_ module arguments to Rhsm.register(), and thus as arguments for subscription-manager register:

  • right before calling Rhsm.register(), Rhsm.configure() is called to configure subscription-manager with all the rhsm_, and server_ arguments; hence, they are already configured
  • the passed argument to --serverurl is partially wrong: Rhsm.register() passes only the hostname, whereas the other bits (port and prefix) are supported too; this "works" because port and prefix were already configured previously, and the lax parsing that subscription-manager does allows for missing bits
  • the parsing done by subscription-manager for --baseurl strips out the URL scheme and always uses https: this means that specifying rhsm_baseurl: http://server as module parameter will be taken as https://server by subscription-manager; since rhsm_baseurl is already configured by Rhsm.configure(), this issue is gone
ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

redhat_subscription

ADDITIONAL INFORMATION

There should be no behaviour change for the module; the only change is when a http URL for rhsm_baseurl is specified, now it is really used as it is.

@ansibullbot
Copy link
Collaborator

cc @alikins @barnabycourt @kahowell
click here for bot help

@ansibullbot ansibullbot added bug This issue/PR relates to a bug module module os packaging plugins plugin (any type) tests tests unit tests/unit labels Nov 18, 2022
@ptoscano ptoscano force-pushed the redhat_subscription-extra-register-args branch from 4c422c8 to f5c35b0 Compare November 18, 2022 20:15
@felixfontein felixfontein added check-before-release PR will be looked at again shortly before release and merged if possible. backport-6 labels Nov 18, 2022
felixfontein
felixfontein reviewed Nov 18, 2022
View reviewed changes
Copy link
Collaborator

@felixfontein felixfontein left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution. Can you please add a changelog fragment? Thanks.

@ptoscano ptoscano force-pushed the redhat_subscription-extra-register-args branch from f5c35b0 to d30f678 Compare November 19, 2022 09:25
@ptoscano ptoscano force-pushed the redhat_subscription-extra-register-args branch from d30f678 to c3d2ad7 Compare November 19, 2022 21:50
@ansibullbot ansibullbot added the stale_ci CI is older than 7 days, rerun before merging label Nov 28, 2022
@ptoscano
redhat_subscription: drop unneeded args to Rhsm.register()
5cc99bb 
Stop passing all the "rhsm_", and "server_" module arguments to
"Rhsm.register()", and thus as arguments for
"subscription-manager register":
- right before calling "Rhsm.register()", "Rhsm.configure()" is called
  to configure subscription-manager with all the "rhsm_", and "server_"
  arguments; hence, they are already configured
- the passed argument to "--serverurl" is partially wrong:
  "Rhsm.register()" passes only the hostname, whereas the other bits
  (port and prefix) are supported too; this "works" because port and
  prefix were already configured previously, and the lax parsing that
  subscription-manager does allows for missing bits
- the parsing done by subscription-manager for "--baseurl" strips out
  the URL scheme and always uses https: this means that specifying
  "rhsm_baseurl: http://server" as module parameter will be taken as
  "https://server" by subscription-manager; since "rhsm_baseurl" is
  already configured by "Rhsm.configure()", this issue is gone
@ptoscano ptoscano force-pushed the redhat_subscription-extra-register-args branch from c3d2ad7 to 5cc99bb Compare November 28, 2022 07:37
@ansibullbot ansibullbot removed the stale_ci CI is older than 7 days, rerun before merging label Nov 28, 2022
@evgeni
Copy link
Contributor

evgeni commented Nov 29, 2022

ciao pino! ;-)

@felixfontein felixfontein removed the check-before-release PR will be looked at again shortly before release and merged if possible. label Nov 29, 2022
@felixfontein felixfontein merged commit 101c957 into ansible-collections:main Nov 29, 2022
@patchback
Copy link

patchback bot commented Nov 29, 2022 •
edited
Loading

Backport to stable-6: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-6/101c957631445dc3902eb71f74b0f52708cc8050/pr-5583

Backported as #5626

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

patchback bot pushed a commit that referenced this pull request Nov 29, 2022
@ptoscano @patchback
redhat_subscription: drop unneeded args to Rhsm.register() (#5583)
e2e164c 
Stop passing all the "rhsm_", and "server_" module arguments to
"Rhsm.register()", and thus as arguments for
"subscription-manager register":
- right before calling "Rhsm.register()", "Rhsm.configure()" is called
  to configure subscription-manager with all the "rhsm_", and "server_"
  arguments; hence, they are already configured
- the passed argument to "--serverurl" is partially wrong:
  "Rhsm.register()" passes only the hostname, whereas the other bits
  (port and prefix) are supported too; this "works" because port and
  prefix were already configured previously, and the lax parsing that
  subscription-manager does allows for missing bits
- the parsing done by subscription-manager for "--baseurl" strips out
  the URL scheme and always uses https: this means that specifying
  "rhsm_baseurl: http://server" as module parameter will be taken as
  "https://server" by subscription-manager; since "rhsm_baseurl" is
  already configured by "Rhsm.configure()", this issue is gone

(cherry picked from commit 101c957)
@patchback patchback bot mentioned this pull request Nov 29, 2022
Merged
@felixfontein
Copy link
Collaborator

@ptoscano thanks for your contribution!
@evgeni thanks for reviewing!

felixfontein pushed a commit that referenced this pull request Nov 29, 2022
@patchback @ptoscano
redhat_subscription: drop unneeded args to Rhsm.register() (#5583) (#…
efedd0d 
…5626)

Stop passing all the "rhsm_", and "server_" module arguments to
"Rhsm.register()", and thus as arguments for
"subscription-manager register":
- right before calling "Rhsm.register()", "Rhsm.configure()" is called
  to configure subscription-manager with all the "rhsm_", and "server_"
  arguments; hence, they are already configured
- the passed argument to "--serverurl" is partially wrong:
  "Rhsm.register()" passes only the hostname, whereas the other bits
  (port and prefix) are supported too; this "works" because port and
  prefix were already configured previously, and the lax parsing that
  subscription-manager does allows for missing bits
- the parsing done by subscription-manager for "--baseurl" strips out
  the URL scheme and always uses https: this means that specifying
  "rhsm_baseurl: http://server" as module parameter will be taken as
  "https://server" by subscription-manager; since "rhsm_baseurl" is
  already configured by "Rhsm.configure()", this issue is gone

(cherry picked from commit 101c957)

Co-authored-by: Pino Toscano <ptoscano@redhat.com>
@ptoscano ptoscano deleted the redhat_subscription-extra-register-args branch November 29, 2022 13:13
ptoscano added a commit to ptoscano/linux-system-roles-rhc that referenced this pull request Jun 1, 2023
@ptoscano
Pass rhc_baseurl directly to redhat_subscription
34cfef4 
The redhat_subscription used to handle certain options both writing them
to the config file (using "subscription-manager config"), and passing
them as command line parameters to "subscription-manager register".
This was problematic for rhsm_baseurl, as subscription-manager sadly
forces "https" as scheme when parsing "--baseurl" as passed to
"register". This was fixed with [1] (backported to community.general 6.x
with [2]), available since community.general 6.1.0.

Hence, directly pass the "rhc_baseurl" role parameter as "rhsm_baseurl"
parameter of the redhat_subscription module.

[1] ansible-collections/community.general#5583
[2] ansible-collections/community.general#5626

Signed-off-by: Pino Toscano <ptoscano@redhat.com>
ptoscano added a commit to ptoscano/linux-system-roles-rhc that referenced this pull request Jun 1, 2023
@ptoscano
refactor: pass rhc_baseurl directly to redhat_subscription
dd69b93 
The redhat_subscription used to handle certain options both writing them
to the config file (using "subscription-manager config"), and passing
them as command line parameters to "subscription-manager register".
This was problematic for rhsm_baseurl, as subscription-manager sadly
forces "https" as scheme when parsing "--baseurl" as passed to
"register". This was fixed with [1] (backported to community.general 6.x
with [2]), available since community.general 6.1.0.

Hence, directly pass the "rhc_baseurl" role parameter as "rhsm_baseurl"
parameter of the redhat_subscription module.

[1] ansible-collections/community.general#5583
[2] ansible-collections/community.general#5626

Signed-off-by: Pino Toscano <ptoscano@redhat.com>
@ptoscano ptoscano mentioned this pull request Jun 1, 2023
Merged
richm pushed a commit to linux-system-roles/rhc that referenced this pull request Jun 1, 2023
@ptoscano @richm
refactor: pass rhc_baseurl directly to redhat_subscription
b95a84e 
The redhat_subscription used to handle certain options both writing them
to the config file (using "subscription-manager config"), and passing
them as command line parameters to "subscription-manager register".
This was problematic for rhsm_baseurl, as subscription-manager sadly
forces "https" as scheme when parsing "--baseurl" as passed to
"register". This was fixed with [1] (backported to community.general 6.x
with [2]), available since community.general 6.1.0.

Hence, directly pass the "rhc_baseurl" role parameter as "rhsm_baseurl"
parameter of the redhat_subscription module.

[1] ansible-collections/community.general#5583
[2] ansible-collections/community.general#5626

Signed-off-by: Pino Toscano <ptoscano@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@felixfontein felixfontein felixfontein left review comments

@evgeni evgeni evgeni approved these changes

Assignees
No one assigned
Labels
bug This issue/PR relates to a bug module module os packaging plugins plugin (any type) tests tests unit tests/unit
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@ptoscano @ansibullbot @evgeni @felixfontein