[WIP] Added CDN configuration to organizations module #1523
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
I tried some more tests, but it seems that I probably need some extra logic to resolve the content credential before putting it in the payload the module sends towards Foreman/Satellite. Also, it always changes right now, is that something I missed? Or doesn't the API report these settings back? @evgeni any ideas? :) |
bittner
suggested changes
Nov 27, 2022
evgeni
reviewed
Nov 30, 2022
evgeni
reviewed
Nov 30, 2022
plugins/modules/organization.py
Outdated
Comment on lines
178
to
190
| # upstream_ca_cert=dict(required=False, type='entity', resource_type='content_credentials', scope=['']), | ||
| upstream_ca_cert_id=dict(required=False), |
There was a problem hiding this comment.
we really shouldn't require the user to provide an id, why did the commented code not work? because of the scope?
in other places, we use it like this:
ssl_ca_cert=dict(type='entity', resource_type='content_credentials', scope=['organization']),There was a problem hiding this comment.
Yes, I fully agree, this is just my current 'worked-around' version, and indeed, because of the scope.
There was a problem hiding this comment.
@evgeni I think at first I copied that line of code from repository.py or something like it, but it kept complaining about that the scope couldn't be found :/
evgeni
reviewed
Nov 30, 2022
|
|
||
| if extra_payload: | ||
| payload.update(extra_payload) | ||
| module.resource_action('organizations', 'cdn_configuration', payload) |
There was a problem hiding this comment.
Do I read it right, this essentially always returns changed=True now, as there is no way to "diff" the cdn config?!
There was a problem hiding this comment.
The data is shown in the API reply to /katello/api/organizations/:id, so we could diff it… What a mess
There was a problem hiding this comment.
I have tried poking around in the API, but I can't find it.
When I go to https://sat.rh.lab/api/organizations/1/cdn_configuration I get a 404. And it's not listed in the 'overview' at the organizations/<org_id> endpoint either.
There was a problem hiding this comment.
attention attention! katello overrides the org controller!
you need to go to /katello/api/organizations/:id, not to /api/organizations/:id, the apipie library will do the right thing for us.
and yes, the cdn_configuration part is only defined for PUT (update) operations, you can't GET it, but the data is part of the normal org show view, if you look at the right controller
There was a problem hiding this comment.
I feel my Python-Fu is lacking a bit, but I made an attempt to compare some things, and it seems to work well :-)
This will never be idempotent for network_sync as the current password cannot be compared with the provided password.
evgeni
force-pushed
the
add_configure_cdn_to_org
branch
from
3c2049b
to
a1a7860
Compare
|
@evgeni ping :-) |
Thulium-Drake
force-pushed
the
add_configure_cdn_to_org
branch
from
b100239
to
b37a65d
Compare
|
@evgeni I've rebased my edits and removed the workaroundy hack with the content_credential_id, this is the debug output when running it: |
evgeni
force-pushed
the
add_configure_cdn_to_org
branch
from
4d7d9b1
to
c5aa500
Compare
evgeni
force-pushed
the
add_configure_cdn_to_org
branch
from
45eba02
to
458f2a8
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
o/ guys,
As mentioned in #1450 , the
foreman_organizationmodule doesn't do CDN configuration.I took a stab at it and tried to integrate it. It seems to work well, but it's still a WIP, but it seems to function (I only tested 'redhat_cdn' with a different URL and 'export_sync', I still need to set up a secondary test Satellite).
Fire away! I'd love feedback!