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

Update API with per provider opts: #143

Merged
merged 6 commits into from
Sep 14, 2023

Conversation

jacobweinstock
Copy link
Member

Description

This enables customization per provider. For example, redfish port, ipmitool cipher suite.

Why is this needed

Once implemented will resolve #89 and #103

Fixes:

How Has This Been Tested?

How are existing users impacted? What migration steps/scripts do we need?

Checklist:

I have:

  • updated the documentation and/or roadmap (if required)
  • added unit or e2e tests
  • provided instructions on how to upgrade

@jacobweinstock jacobweinstock added the kind/feature New feature or request label Sep 1, 2023
@codecov
Copy link

codecov bot commented Sep 1, 2023

Codecov Report

Merging #143 (2edcded) into main (bda0103) will decrease coverage by 0.22%.
The diff coverage is 50.00%.

❗ Current head 2edcded differs from pull request most recent head 950d970. Consider uploading reports for the commit 950d970 to get more accurate results

@@            Coverage Diff             @@
##             main     #143      +/-   ##
==========================================
- Coverage   62.03%   61.82%   -0.22%     
==========================================
  Files           4        4              
  Lines         353      351       -2     
==========================================
- Hits          219      217       -2     
  Misses        100      100              
  Partials       34       34              
Files Changed Coverage Δ
controllers/machine.go 79.77% <50.00%> (-0.45%) ⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@jacobweinstock
Copy link
Member Author

Hey Fellas, @chrisdoherty4 , @joelrebel , @pokearu . When you have some cycles, I'd appreciate a review. Please and thank you!

api/v1alpha1/machine.go Outdated Show resolved Hide resolved
api/v1alpha1/machine.go Show resolved Hide resolved
api/v1alpha1/machine.go Outdated Show resolved Hide resolved
api/v1alpha1/provider_opts.go Outdated Show resolved Hide resolved
api/v1alpha1/machine.go Outdated Show resolved Hide resolved
This enables customization per provider.
For example, redfish port, ipmitool cipher suite.

Signed-off-by: Jacob Weinstock <[email protected]>
Signed-off-by: Jacob Weinstock <[email protected]>
Signed-off-by: Jacob Weinstock <[email protected]>
The way the linter was run the exit code was
being swallowed up and always return 0. This
meant that even with linting issues could CI
was passing. This fixes that.

The new version of the linter found some unused
functions, so they were removed. The other changes
were to resolve linting issues that were found.

Signed-off-by: Jacob Weinstock <[email protected]>
Copy link
Member

@chrisdoherty4 chrisdoherty4 left a comment

Choose a reason for hiding this comment

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

lgtm

// PrefixSigDisabled determines whether the algorithm will be prefixed to the signature. Example: sha256=abc123
PrefixSigDisabled bool `json:"prefixSigDisabled"`
// Secrets are a map of algorithms to secrets used for signing.
Secrets RPCSecrets `json:"secrets"`
Copy link
Member

Choose a reason for hiding this comment

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

Might consider HMACSecrets

@jacobweinstock jacobweinstock added the ready-to-merge Mergify: Ready for Merging label Sep 14, 2023
@jacobweinstock
Copy link
Member Author

@Mergifyio queue

@mergify
Copy link
Contributor

mergify bot commented Sep 14, 2023

queue

✅ The pull request has been merged automatically

The pull request has been merged automatically at 357b551

@mergify mergify bot merged commit 357b551 into tinkerbell:main Sep 14, 2023
6 checks passed
@jacobweinstock jacobweinstock deleted the provider-opts branch September 14, 2023 16:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature New feature or request ready-to-merge Mergify: Ready for Merging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Question: Is there any plan to support to set a specific cipher suite for ipmi?
2 participants