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

[1/4] public library: Implement HTTP connection #278

Merged
merged 1 commit into from
Jan 15, 2025
Merged

Conversation

felixsch
Copy link
Contributor

@felixsch felixsch commented Jan 2, 2025

related card: https://trello.com/c/uRZTaz4g/3682-rr1-library-implement-announce-activate-de-activate-and-de-register

This is the public library implementation which is required to allow Rancher Manager to register against SCC.
The general idea is to easily allow products to register themself to SCC and therefore show up as activated product in the systems details.

In this merge request:

  • Add HTTP connection implementation
  • Add Credentials handling
  • Prepare public-api-test binary to act as demo tool

About the pull requests

There are multiple merge requests which all together implement a useful subset of the overall API functionality of SCC.
[1/4] public library: Implement HTTP connection
[2/4] public library: Implement registration and status/keepalive
[3/4] public library: Implement product activation and extension traversal
[4/4] public library: repository cleanup

I you are here to review these, please start with the first one (this one) and only start with the next merge request when this one is merged. I will update bases accordingly then. Each merge request enhances the public-api-test binary to show demo usage of the implemented functionality in this merge request.

If you want to see a full CI run of this set checkout: #281

How to review this merge request (and all others):

  • Checkout the branch and make sure the documentation looks sane. You can render the documentation using:
$ go install golang.org/x/pkgsite/cmd/pkgsite@latest
$ pkgsite -open .
  • Run the public-api-test application including a demo showing the implementation working
$ make build
$ REGCODE=<regcode> ./out/public-api-test SLES 15.5 x86_64
  • Check if you can spot inconsistencies regarding the implementation. Any areas of improvements?

If you have any questions, need help setting up development environment, do not hesitate to reach out to me 🚀

Thank you! ❤️

pkg/connection/connection.go Outdated Show resolved Hide resolved
pkg/connection/credentials.go Show resolved Hide resolved
pkg/connection/connection.go Outdated Show resolved Hide resolved
cmd/public-api-test/credentials.go Show resolved Hide resolved
cmd/public-api-test/credentials.go Outdated Show resolved Hide resolved
cmd/public-api-test/credentials.go Outdated Show resolved Hide resolved
pkg/connection/auth.go Outdated Show resolved Hide resolved
pkg/connection/auth.go Outdated Show resolved Hide resolved
pkg/connection/connection.go Outdated Show resolved Hide resolved
pkg/connection/connection.go Show resolved Hide resolved
pkg/connection/connection.go Outdated Show resolved Hide resolved
Copy link
Contributor

@mssola mssola left a comment

Choose a reason for hiding this comment

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

This looks great! To be honest I only have two major concerns before merging:

  1. The version file, as explained on a comment.
  2. This PR includes two commit, yours and mine. We can squash them as mine was meant more of a WIP thingie. Hence, before merging, let us squash it with your commit message instead.

Makefile Outdated Show resolved Hide resolved
cmd/public-api-test/credentials.go Outdated Show resolved Hide resolved
pkg/connection/connection.go Outdated Show resolved Hide resolved
pkg/connection/connection.go Outdated Show resolved Hide resolved
pkg/connection/connection.go Show resolved Hide resolved
pkg/connection/credentials.go Show resolved Hide resolved
@mssola mssola force-pushed the connect-public-library branch 2 times, most recently from d616018 to fd80c1c Compare January 9, 2025 11:39
Introduce several interfaces which allow a caller to perform API
requests against an SCC-like API. This includes:

* Common options for the connection
* Credentials handling
* Building and performing requests
* Automatically handle system token rotations

Reviewed-by: Parag Jain <[email protected]>
Signed-off-by: Felix Schnizlein <[email protected]>
Signed-off-by: Miquel Sabaté Solà <[email protected]>
@mssola mssola force-pushed the connect-public-library branch from fd80c1c to 3fb6984 Compare January 9, 2025 11:41
@mssola
Copy link
Contributor

mssola commented Jan 9, 2025

@felixsch @paragjain0910 updated the PR with all the suggestions. PTAL

Copy link
Contributor Author

@felixsch felixsch left a comment

Choose a reason for hiding this comment

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

lgtm 👍

I'm still not sold on the collectors.Result topic but lets discuss in the next PR

@mssola
Copy link
Contributor

mssola commented Jan 14, 2025

I'm still not sold on the collectors.Result topic but lets discuss in the next PR

Yes, let's see it on the next PRs and check what makes sense.

@mssola mssola merged commit 7949fd7 into main Jan 15, 2025
2 checks passed
@mssola mssola deleted the connect-public-library branch January 15, 2025 06:27
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.

3 participants