Fixes #33783 - Support connected Inter-Server Sync #8821

jturel · 2020-07-08T12:54:38Z

What are the changes introduced in this pull request?

This change brings back the ability to sync Red Hat content between two Katello instances. There was a way to achieve this when we were on pulp2 but that went away due to the lack of pulp3 publishing a file structure to disk that mimics the Red Hat CDN.

It works by introducing a new CdnConfiguration model which allows the user to set the necessary information to talk to the API of another Katello. That information is used to configure a new KatelloCdn resource that extends the existing CdnResource class. By providing similar (and some new) APIs, the KatelloCdn can be used by the /repository_sets/:id/available_repositories API to know which repositories are available for enablement, and properly configure any enabled repo to pull content from the upstream instance by using the org's debug certificate as the auth mechanism.

What are the testing steps for this pull request?

Testing this requires two Katello instances- a devel box and a 4.2 box for example. Only the downstream box (devel) needs to have this change applied which also makes this feature portable to older Katellos using pulp3.

I used the following script in my own testing to execute the workflow with two orgs on a single Katello instance and it could be adapted for two boxes if you want: https://gist.github.com/jturel/2e18c3784f3abeefcbfb3905f5372e66

The workflow is:

Upstream Katello setup

Configure some Red Hat content. => enable a small repo like satellite-tools
Create a personal access token for a user who would have access to the repository sets API (could be admin...)

Downstream Katello setup

Apply this code change, migrate DB
Import a manifest which provides the same repo you enabled above
Create a Content Credential which has the CA certificate of the upstream server
Update the CdnConfiguration of your downstream org to talk to the upstream server:

curl http://localhost:3000/katello/api/v2/organizations/$ORG_ID/cdn_configuration -u admin:changeme -X PUT -H 'Content-type: application/json' \
  -d '{"username": "$UPSTREAM_KATELLO_USER'", "password": "$PERSONAL_ACCESS_TOKEN", "organization_label": "'$UPSTREAM_KATELLO_ORG_LABEL", "url": "https://$UPSTREAM_KATELLO_FQDN", "ssl_ca_credential_id": "$CONTENT_CREDENTIAL_ID"}'

Browse the Red Hat repositories of the downstream Katello. Expanding any repo set should call back to the API of your upstream server to fetch the available paths. If you get an error the CdnConfiguration is likely wrong. If it works - good job following my instructions so far :)
Try enabling a repository which is not enabled on the upstream server. You should get an intelligible error.
Now enable the same repository that you enabled on the upstream server at the start. It should be enable-able and you should be able to sync it. Check the traffic on your upstream server to verify. Also check the repository details and note that the upstream URL uses a path from the upstream instance rather than cdn.redhat.com

What I really want you to test

Analyze my design and understand how it works. Where does it fail?
Are there any workflows you can execute that give a bad user experience or just don't work?
Check the inline comments I've left on the code
What happens when you revert the downstream CdnConfiguration ie set everything to empty? Can you successfully sync from the Red Hat CDN again?

jeremylenz

Thanks for doing this investigation work @jturel

Seems really well thought-out and straightforward. Overall it really does make a lot of sense to me, nice work! After reading the doc I think I understand (mostly)

some questions:

The general gist of this seems to be "If we're using the RedHat CDN, carry on as normal. But if the CDN is another satellite, do lots of certificate magic and then 2 API requests." Right? Could it be done in a single API request?
What is a "debug" certificate normally used for? Is there something unorthodox about using them in this way?

jturel · 2020-08-03T20:29:41Z

Thanks for doing this investigation work @jturel

Seems really well thought-out and straightforward. Overall it really does make a lot of sense to me, nice work! After reading the doc I think I understand (mostly)

some questions:

The general gist of this seems to be "If we're using the RedHat CDN, carry on as normal. But if the CDN is another satellite, do lots of certificate magic and then 2 API requests." Right? Could it be done in a single API request?

Hopefully not too much magic. Just take the certs of the upstream Satellite, create Content Credentials for them, and associate to your 'CDNConfiguration' which will manifest on the Manage Manifest modal.

Technically, it could be done with a single API call, but I think that would mean we need a new API. The approach here really keeps it simple by staying true to our current API. That said, I'll give it another hard look when I pick this back up (next sprint hopefully).

What is a "debug" certificate normally used for? Is there something unorthodox about using them in this way?

A debug certificate (which can be retrieved over the API -> /organizations/:id/debug_certificate) provides a way for folks to get content off their Katello without registering a content host, I guess for experimental purposes. There are probably other uses I'm not thinking of. I think it's fair to say this is a bit unorthodox because we're bringing these certs into real workflow. At the same time it's a great fit, and I don't think we're getting too coupled to the certs. We could move to some other auth mechanism in the future.

jlsherrill · 2021-08-16T15:38:41Z

app/lib/actions/katello/repository_set/available_repositories.rb

+          results = []
+          cdn_config = product.organization.cdn_configuration
+
+          if ::Katello::Resources::CDN::CdnResource.redhat_cdn?(cdn_config.url)


One consequence of this change is that it will assume anything that isn't 'cdn.redhat.com' is another katello instance. So something like cdn.stage.redhat.com or a manually configured http:// cdn would not work i presume (although stage is maybe the better use case). I guess the user could manually set the SETTING to their url, but i'm not sure the installer supports that. Maybe having a cdn type field or other configuration would help make this clear cut?

This should be addressed in the latest revision. See #redhat? method on CdnConfiguration model

jturel · 2021-10-27T14:49:01Z

app/lib/katello/resources/cdn.rb

@@ -93,8 +109,13 @@ def get(path, _headers = {})
          net = http_downloader
          path = File.join(@uri.request_uri, path)
          used_url = File.join("#{@uri.scheme}://#{@uri.host}:#{@uri.port}", path)
-          Rails.logger.debug "CDN: Requesting path #{used_url}"
+          Rails.logger.info "CDN: Requesting path #{used_url}"


I think this is used infrequently enough to warrant info level logs. It could help debugging

jturel · 2021-10-27T14:50:10Z

app/lib/katello/util/path_with_substitutions.rb

      end

-      def resolve_substitutions(cdn_resource)


I needed to change CdnVarSubstitutor and I got looking into this code. I thought it shouldn't be aware of any CdnResource object so I refactoried it out and into CdnVarSubstitutor

jturel · 2021-10-27T14:50:34Z

app/models/katello/cdn_configuration.rb

+
+    # test deleting the CA Cred...
+    belongs_to :ssl_ca_credential, :class_name => "Katello::ContentCredential", :inverse_of => :ssl_ca_cdn_configurations
+


I think I should definitely add some validations here.

Oh as in fail if content credential is not Provided and its not a redhat ?

See below block of validations!

parthaa

Works well for me. ACK

jlsherrill · 2021-11-01T17:27:17Z

db/migrate/20211019192121_create_cdn_configuration.katello.rb

+        url: org.redhat_provider.repository_url || ::Katello::Resources::CDN::CdnResource.redhat_cdn_url
+      ).first_or_create!
+    end
+  end


should we drop the redhat provider url column here?

Good idea, but I I'd like to file a redmine and do it separately. Reason being is that this code might get ported back to 6.9 perhaps as a hotfix for some customers who want to sync RH content from their 7.0. In order to limit the size+risk of the change, and especially because the OrganizationCreator (thing that replaced the Org::Create dynflow task) isn't in 6.9 and moved creation of providers etc. around, I'd rather have the unused field left over in the code+db than break something. Lemme know if I'm not making any sense :)

Taking a look - maybe it's not so bad

jlsherrill · 2021-11-01T17:27:44Z

overall design looks good, couple of small questions

jturel · 2021-11-02T00:07:49Z

[test katello]

jturel · 2021-11-02T16:03:43Z

@jlsherrill updated! take a look at the top few commits

jturel · 2021-11-02T16:07:45Z

Ooops, fixing something :)

jturel · 2021-11-02T16:14:18Z

Fixed!

jturel · 2021-11-02T17:24:54Z

app/controllers/katello/api/v2/subscriptions_controller.rb

@@ -108,8 +108,7 @@ def upload

      # repository url
      if (repo_url = params[:repository_url])
-        @provider.repository_url = repo_url
-        @provider.save!
+        @organization.cdn_configuration.update!(url: repo_url)


@parthaa @jlsherrill I wonder if we should remove the repository_url param from this API as part of this change? My thinking is that this code doesn't run the CdnConfiguration::Update (previously Provider::Update) action when making the change - so I don't see its purpose. Notice that the organizations controller does run the dynflow action - and that's what the existing UI uses.

The API param doesn't show up currently in hammer or foreman-documentation repo so it should be safe

i think it would be fine to remove it entirely, since its broken as is

jturel · 2021-11-02T20:20:37Z

[test katello]

jlsherrill

ACK!

theforeman-bot added Waiting on contributor Needs testing Not yet reviewed labels Jul 8, 2020

jturel force-pushed the inter_server_sync branch from fc6ae04 to 85ab94d Compare July 9, 2020 18:06

theforeman-bot added Waiting on contributor and removed Waiting on contributor labels Jul 9, 2020

jeremylenz reviewed Aug 3, 2020

View reviewed changes

Base automatically changed from master to main January 28, 2021 17:16

Base automatically changed from main to master January 29, 2021 03:46

jlsherrill reviewed Aug 16, 2021

View reviewed changes

jturel force-pushed the inter_server_sync branch from 85ab94d to da627ab Compare August 20, 2021 15:19

theforeman-bot added Waiting on contributor and removed Waiting on contributor labels Aug 20, 2021

jturel force-pushed the inter_server_sync branch from 38e1634 to b94bf79 Compare October 25, 2021 23:11

theforeman-bot removed the Waiting on contributor label Oct 25, 2021

jturel changed the title ~~Inter-Server Sync concept~~ Fixes #33783 - Support connected Inter-Server Sync Oct 25, 2021

jturel force-pushed the inter_server_sync branch from b94bf79 to f31ff0d Compare October 25, 2021 23:20

jturel marked this pull request as ready for review October 25, 2021 23:21

jturel added 3 commits October 26, 2021 18:28

Fixes #33783 - Support connected Inter-Server Sync

36f9e35

Refs #33783 - fix listing available repos

5516f0f

Refs #33783 - cleanup + test fixes

a55d98d

jturel force-pushed the inter_server_sync branch from f31ff0d to a55d98d Compare October 26, 2021 19:49

jturel commented Oct 27, 2021

View reviewed changes

parthaa approved these changes Oct 29, 2021

View reviewed changes

jlsherrill reviewed Nov 1, 2021

View reviewed changes

Refs #33783 - clean up Provider repository_url

e4f32f5

theforeman-bot added Waiting on contributor and removed Waiting on contributor labels Nov 1, 2021

Refs #33783 - clean up Provider tests

df48c0d

theforeman-bot added Waiting on contributor and removed Waiting on contributor labels Nov 1, 2021

Refs #33783 - make validation errors more clear for non-red hat

e0a143b

theforeman-bot added Waiting on contributor and removed Waiting on contributor labels Nov 2, 2021

Refs #33783 - fix validation

bbaf06c

theforeman-bot added Waiting on contributor and removed Waiting on contributor labels Nov 2, 2021

jturel commented Nov 2, 2021

View reviewed changes

Refs #33783 - remove repository_url from manifest upload API

63b614f

theforeman-bot added Waiting on contributor and removed Waiting on contributor labels Nov 2, 2021

jlsherrill approved these changes Nov 3, 2021

View reviewed changes

jturel merged commit d40c039 into Katello:master Nov 3, 2021

jturel deleted the inter_server_sync branch November 3, 2021 21:44

jturel mentioned this pull request Nov 8, 2021

Fixes #33874 - UI for Connected ISS #9772

Merged

1 task

parthaa mentioned this pull request Dec 14, 2021

Fixes #34144 - Connected ISS from a CVE #9853

Merged

Gauravtalreja1 mentioned this pull request Apr 7, 2022

Update expected API endpoints for 6.11 SatelliteQE/robottelo#9485

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixes #33783 - Support connected Inter-Server Sync #8821

Fixes #33783 - Support connected Inter-Server Sync #8821

jturel commented Jul 8, 2020 •

edited

jeremylenz left a comment

jturel commented Aug 3, 2020

jlsherrill Aug 16, 2021

jturel Oct 27, 2021

jturel Oct 27, 2021

jturel Oct 27, 2021

jturel Oct 27, 2021

parthaa Oct 28, 2021

jturel Oct 28, 2021

parthaa left a comment

jlsherrill Nov 1, 2021

jturel Nov 1, 2021 •

edited

jturel Nov 1, 2021

jlsherrill commented Nov 1, 2021

jturel commented Nov 2, 2021

jturel commented Nov 2, 2021

jturel commented Nov 2, 2021

jturel commented Nov 2, 2021

jturel Nov 2, 2021

jturel Nov 2, 2021

jlsherrill Nov 2, 2021

jturel commented Nov 2, 2021

jlsherrill left a comment


		# test deleting the CA Cred...
		belongs_to :ssl_ca_credential, :class_name => "Katello::ContentCredential", :inverse_of => :ssl_ca_cdn_configurations

Fixes #33783 - Support connected Inter-Server Sync #8821

Fixes #33783 - Support connected Inter-Server Sync #8821

Conversation

jturel commented Jul 8, 2020 • edited

What are the changes introduced in this pull request?

What are the testing steps for this pull request?

jeremylenz left a comment

Choose a reason for hiding this comment

jturel commented Aug 3, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

parthaa left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jturel Nov 1, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jlsherrill commented Nov 1, 2021

jturel commented Nov 2, 2021

jturel commented Nov 2, 2021

jturel commented Nov 2, 2021

jturel commented Nov 2, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jturel commented Nov 2, 2021

jlsherrill left a comment

Choose a reason for hiding this comment

jturel commented Jul 8, 2020 •

edited

jturel Nov 1, 2021 •

edited