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

Use cacert from rancher and use serverl-url from rancher #36

Merged
merged 6 commits into from
Jul 13, 2022

Conversation

Itxaka
Copy link
Contributor

@Itxaka Itxaka commented Jul 13, 2022

Signed-off-by: Itxaka igarcia@suse.com

Fixes #32

Itxaka added 2 commits July 13, 2022 10:38
Signed-off-by: Itxaka <igarcia@suse.com>
Also drops the cacert passing via chart, adds proper permissions to the
settings resource, adds the cache index and checks that indeed we are
returning a cacert on the registration url

Signed-off-by: Itxaka <igarcia@suse.com>
@Itxaka Itxaka requested review from kkaempf, mudler, fgiudici and a team July 13, 2022 11:55
@Itxaka Itxaka changed the title Generate v3.Setting code Use cacert from rancher Jul 13, 2022
@codecov-commenter
Copy link

Codecov Report

Merging #36 (73df0fb) into main (bcfe4d0) will decrease coverage by 1.13%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##             main      #36      +/-   ##
==========================================
- Coverage   42.55%   41.42%   -1.14%     
==========================================
  Files           9        9              
  Lines         658      676      +18     
==========================================
  Hits          280      280              
- Misses        358      376      +18     
  Partials       20       20              
Impacted Files Coverage Δ
pkg/server/register.go 13.71% <0.00%> (ø)
pkg/server/server.go 0.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bcfe4d0...73df0fb. Read the comment docs.

Signed-off-by: Itxaka <igarcia@suse.com>
Copy link
Contributor

@frelon frelon left a comment

Choose a reason for hiding this comment

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

Very nice!

Copy link
Contributor

@davidcassany davidcassany left a comment

Choose a reason for hiding this comment

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

Cool!

Copy link
Contributor

@mjura mjura left a comment

Choose a reason for hiding this comment

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

LGTM

@Itxaka Itxaka added the Blocked label Jul 13, 2022
@Itxaka
Copy link
Contributor Author

Itxaka commented Jul 13, 2022

sorry folks, Im gonna actually update this to include the server-url fix as its pretty simple and similar!

Itxaka added 2 commits July 13, 2022 15:05
Drop any manual setup of rancher-url and use the settings to get the set
rancher-url

Signed-off-by: Itxaka <igarcia@suse.com>
Signed-off-by: Itxaka <igarcia@suse.com>
@Itxaka Itxaka changed the title Use cacert from rancher Use cacert from rancher and use serverl-url from rancher Jul 13, 2022
@Itxaka Itxaka requested review from mjura, davidcassany and frelon July 13, 2022 13:07
Copy link
Contributor

@frelon frelon left a comment

Choose a reason for hiding this comment

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

Even better 👍

Copy link
Contributor

@mjura mjura left a comment

Choose a reason for hiding this comment

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

Good job 👌

Signed-off-by: Itxaka <igarcia@suse.com>
@Itxaka Itxaka force-pushed the cacerts_from_rancher branch from 5c9e9eb to cfcd5b7 Compare July 13, 2022 13:19
@Itxaka Itxaka merged commit e87eb80 into main Jul 13, 2022
@fgiudici fgiudici removed the Blocked label Jul 22, 2022
@ldevulder ldevulder deleted the cacerts_from_rancher branch August 3, 2022 12:17
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.

CACert is missing and registrationURL it's "incomplete" from the MachineInventory status spec
6 participants