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

Support arbitrary registration URL #610

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

josegomezr
Copy link

Add support for operatingSystem.packages.sccRegistrationUrl to allow for registration against an RMT/SMT/SUMA.

Similar to suse-edge/misc#112

@atanasdinov
Copy link
Contributor

Thank you for your contribution, however, I have to note that this registration only concerns the intermediate container used for RPM resolution, and not the final provisioned node(s).

What is the problem that we're trying to solve?

@josegomezr
Copy link
Author

Enable the full process to be able to talk with a SUMA/RMT/SCC Instance.

I understand that this container calls suseconnect to get repositories added via the services returned by the registration server (scc right now) and then will download all RPMs (from the default location: CDN).

By allowing the registration URL just before the download, the downloads can be served by SUMA/RMT/SMT instead.

Or at least is my understanding on when each stage is called 😅

@jdob
Copy link
Contributor

jdob commented Nov 20, 2024

This is awesome, thanks for the contribution. I'll do a quick test on my own to verify the templating part works correctly. In the meantime, I have a few notes on areas that aren't necessarily obvious that need to be updated before it can land:

  • pkg/image/validation/os.go - I'd like to see some sort of validation on the URL. Nothing major, but you can use some built in URL functions to make sure it's valid.
  • pkg/image/testdata/full-valid-example.yaml - The intention there is to mention every attribute to demonstrate its usage and be able to unit test the parsing. Please add an entry there (line 78 has a usage of sccRegistrationCode, you can put it under there).
  • pkg/image/definition_test.go - Make sure to update this with the assertion that the new field is parsed correctly.
  • docs/building-images.md - This is our documentation for the definition file. Please update as appropriate (you can follow the model for sccRegistrationCode).
  • RELEASE_NOTES.md - Under "Image Definition Changes", add an entry for this new field with a brief description.

@jdob
Copy link
Contributor

jdob commented Nov 20, 2024

@fdegir This is an outside contribution, so not on our roadmap, but can you put this on your todo list of things to add to CI?

@jdob
Copy link
Contributor

jdob commented Nov 20, 2024

Light testing looked good to me. I don't have a local server to test against, but I was able to get the resolution script to show:

suseconnect -r INTERNAL-USE-ONLY-LOL-NO --url "http://foo"

@@ -12,7 +12,7 @@ set -euo pipefail
# Arch - sets the architecture of the rpm packages to pull
Copy link
Contributor

Choose a reason for hiding this comment

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

One more thing @josegomezr, please add mention of the new template variable and a brief description here.

@rdoxenham
Copy link
Contributor

Thanks for the contribution @josegomezr - another thing to mention in the documentation would be that by default we register against SCC/CDN, and that if the URL is omitted, this what we use by default. Thanks!

josegomezr and others added 6 commits November 22, 2024 21:59
Add support for `operatingSystem.packages.sccRegistrationUrl` to allow
for registration against an RMT/SMT/SUMA.
Use net/url.Parse function to ensure a valid url is present
@josegomezr josegomezr force-pushed the support_registration_url branch from 8b28958 to 5bf8166 Compare November 22, 2024 20:59
Comment on lines +204 to +207
* `sccRegistrationUrl` - Specifies a registration server like RMT or SUMA, which is used to connect download SUSE's internal RPM repositories. Defaults to `https://scc.suse.com`.

> **_NOTE:_** When using `sccRegistrationUrl` over `https` it's important that the `eib` recognizes the CA of the `registrationUrl`.
> This is common on RMT setups. See the [RMT docs on configuring clients](https://documentation.suse.com/sles/15-SP6/html/SLES-all/cha-rmt-client.html).
Copy link
Author

Choose a reason for hiding this comment

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

I must confess I'm not super inspired on this writing part, I'm very open to suggestions 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants