kubeadm: Graduate certs phase #70331
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,18 +20,18 @@ import ( | |
| "fmt" | ||
| "strings" | ||
|
|
||
| "github.com/pkg/errors" | ||
| "github.com/spf13/cobra" | ||
|
|
||
| kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm" | ||
| kubeadmscheme "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/scheme" | ||
| kubeadmapiv1beta1 "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/v1beta1" | ||
| certscmdphase "k8s.io/kubernetes/cmd/kubeadm/app/cmd/phases/certs" | ||
| "k8s.io/kubernetes/cmd/kubeadm/app/cmd/phases/workflow" | ||
| cmdutil "k8s.io/kubernetes/cmd/kubeadm/app/cmd/util" | ||
| kubeadmconstants "k8s.io/kubernetes/cmd/kubeadm/app/constants" | ||
| certsphase "k8s.io/kubernetes/cmd/kubeadm/app/phases/certs" | ||
| kubeadmutil "k8s.io/kubernetes/cmd/kubeadm/app/util" | ||
| "k8s.io/kubernetes/pkg/util/normalizer" | ||
| ) | ||
|
|
||
|
|
@@ -66,6 +66,24 @@ var ( | |
| ` + cmdutil.AlphaDisclaimer) | ||
| ) | ||
|
|
||
| // certsData defines the behavior that a runtime data struct passed to the certs phase should | ||
| // have. Please note that we are using an interface in order to make this phase reusable in different workflows | ||
| // (and thus with different runtime data struct, all of them requested to be compliant to this interface) | ||
| type certsData interface { | ||
| Cfg() *kubeadmapi.InitConfiguration | ||
| CertificateDir() string | ||
| CertificateWriteDir() string | ||
| } | ||
|
|
||
| // NewCertsPhase returns the phase for the certs | ||
| func NewCertsPhase() workflow.Phase { | ||
| return workflow.Phase{ | ||
| Name: "certs", | ||
| Short: "Certificate generation", | ||
| Phases: getCertsSubPhases(), | ||
| } | ||
| } | ||
|
|
||
| // NewCmdCerts returns main command for certs phase | ||
| func NewCmdCerts() *cobra.Command { | ||
|
There was a problem hiding this comment. we don't need both a "cert" phase and a command. the phase will create/handle the command creation. There was a problem hiding this comment. I kept this for the renewal, it's the only thing it contains, and it shouldn't go in the init workflow. There was a problem hiding this comment. i think the renew part needs to be synced with @fabriziopandini 's PR here: maybe this PR should not touch it, but rather the other PR should perform the move. There was a problem hiding this comment. Will have a look on how to avoid this change at all so I don't introduce conflicts in #70333. @fabriziopandini how do you feel about this change? Should I avoid it completely? There was a problem hiding this comment. @ereslibre I'm fine with the current proposed implementation. There was a problem hiding this comment. I removed all changes related to this on this PR |
||
| cmd := &cobra.Command{ | ||
|
|
@@ -75,83 +93,72 @@ func NewCmdCerts() *cobra.Command { | |
| Long: cmdutil.MacroCommandLongDescription, | ||
| } | ||
|
|
||
| cmd.AddCommand(getCertsSubCommands()...) | ||
| return cmd | ||
| } | ||
|
|
||
| // getCertsSubCommands returns sub commands for certs phase | ||
| func getCertsSubCommands() []*cobra.Command { | ||
| return []*cobra.Command{certscmdphase.NewCmdCertsRenewal()} | ||
| } | ||
|
|
||
| // getCertsSubPhases returns sub phases for certs phase | ||
| func getCertsSubPhases() []workflow.Phase { | ||
| subPhases := []workflow.Phase{} | ||
|
|
||
| certTree, _ := certsphase.GetDefaultCertList().AsMap().CertTree() | ||
|
|
||
| for ca, certList := range certTree { | ||
| caPhase := newCertSubPhase(ca, runCAPhase(ca)) | ||
| subPhases = append(subPhases, caPhase) | ||
|
|
||
| for _, cert := range certList { | ||
| certPhase := newCertSubPhase(cert, runCertPhase(cert, ca)) | ||
| subPhases = append(subPhases, certPhase) | ||
| } | ||
|
|
||
| subPhases = append(subPhases, caPhase) | ||
| } | ||
|
|
||
| // SA creates the private/public key pair, which doesn't use x509 at all | ||
| saPhase := workflow.Phase{ | ||
| Name: "sa", | ||
| Short: "Generates a private key for signing service account tokens along with its public key", | ||
| Long: saKeyLongDesc, | ||
| Run: runCertsSa, | ||
| } | ||
|
|
||
| subPhases = append(subPhases, saPhase) | ||
|
|
||
| return subPhases | ||
| } | ||
|
|
||
| func runCertsSa(c workflow.RunData) error { | ||
| data, ok := c.(certsData) | ||
| if !ok { | ||
| return errors.New("certs phase invoked with an invalid data struct") | ||
| } | ||
|
|
||
| cfg := data.Cfg() | ||
| cfg.CertificatesDir = data.CertificateWriteDir() | ||
| defer func() { cfg.CertificatesDir = data.CertificateDir() }() | ||
|
|
||
| return certsphase.CreateServiceAccountKeyAndPublicKeyFiles(cfg) | ||
| } | ||
|
|
||
| func newCertSubPhase(certSpec *certsphase.KubeadmCert, run func(c workflow.RunData) error) workflow.Phase { | ||
| phase := workflow.Phase{ | ||
| Name: certSpec.Name, | ||
| Short: fmt.Sprintf("Generates the %s", certSpec.LongName), | ||
| Long: fmt.Sprintf( | ||
| genericLongDesc, | ||
| certSpec.LongName, | ||
| certSpec.BaseName, | ||
| getSANDescription(certSpec), | ||
| ), | ||
| Run: run, | ||
| } | ||
| return phase | ||
| } | ||
|
|
||
| func getSANDescription(certSpec *certsphase.KubeadmCert) string { | ||
|
|
@@ -188,51 +195,32 @@ func getSANDescription(certSpec *certsphase.KubeadmCert) string { | |
| return fmt.Sprintf("\n\nDefault SANs are %s", strings.Join(sans, ", ")) | ||
| } | ||
|
|
||
| func runCAPhase(ca *certsphase.KubeadmCert) func(c workflow.RunData) error { | ||
| return func(c workflow.RunData) error { | ||
| data, ok := c.(certsData) | ||
| if !ok { | ||
| return errors.New("certs phase invoked with an invalid data struct") | ||
| } | ||
|
|
||
| cfg := data.Cfg() | ||
| cfg.CertificatesDir = data.CertificateWriteDir() | ||
| defer func() { cfg.CertificatesDir = data.CertificateDir() }() | ||
|
|
||
| return certsphase.CreateCACertAndKeyFiles(ca, cfg) | ||
| } | ||
| } | ||
|
|
||
| func runCertPhase(cert *certsphase.KubeadmCert, caCert *certsphase.KubeadmCert) func(c workflow.RunData) error { | ||
| return func(c workflow.RunData) error { | ||
| data, ok := c.(certsData) | ||
| if !ok { | ||
| return errors.New("certs phase invoked with an invalid data struct") | ||
| } | ||
|
|
||
| cfg := data.Cfg() | ||
| cfg.CertificatesDir = data.CertificateWriteDir() | ||
| defer func() { cfg.CertificatesDir = data.CertificateDir() }() | ||
|
|
||
| return certsphase.CreateCertAndKeyFilesWithCA(cert, caCert, cfg) | ||
| } | ||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The UX generation for subcommands is kind of fragmented:
getCertsSubPhases,makePhase,makePhaseForCert,makePhasesForCAIMO, as soon as run functions will be moved into functions as suggested below, the above set of functions can be simplified...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated