Skip to content

🐛 Make joinConfiguration.discovery.bootstrapToken.token optional #12107

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Apr 28, 2025
Merged

🐛 Make joinConfiguration.discovery.bootstrapToken.token optional #12107

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions bootstrap/kubeadm/api/v1beta1/kubeadm_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -584,10 +584,10 @@ type Discovery struct {
type BootstrapTokenDiscovery struct {
Copy link
Contributor

Choose a reason for hiding this comment

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

This object now has no required field, so there will be difference in behaviour between not specifying the object, and specifying bootstrapToken: {}.

This is generally not desirable.

Is it valid to have an empty bootstrapToken? In the current use case, which alternative field would be used instead?

Copy link
Contributor

Choose a reason for hiding this comment

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

From code reading what I can see is that the both the scenario's are handled currently, If boostStrapToken is not set and its nil, its being initialized first.
Also we have necessary checks for respecting the already set values as well.
Reference:

cluster-api/bootstrap/kubeadm/internal/controllers/kubeadmconfig_controller.go

Lines 1242 to 1284 in 3e6bc6a

// otherwise it is necessary to ensure token discovery is properly configured
if config.Spec.JoinConfiguration.Discovery.BootstrapToken == nil {
config.Spec.JoinConfiguration.Discovery.BootstrapToken = &bootstrapv1.BootstrapTokenDiscovery{}
}
// calculate the ca cert hashes if they are not already set
if len(config.Spec.JoinConfiguration.Discovery.BootstrapToken.CACertHashes) == 0 {
hashes, err := certificates.GetByPurpose(secret.ClusterCA).Hashes()
if err != nil {
log.Error(err, "Unable to generate Cluster CA certificate hashes")
return ctrl.Result{}, err
}
config.Spec.JoinConfiguration.Discovery.BootstrapToken.CACertHashes = hashes
}
// if BootstrapToken already contains an APIServerEndpoint, respect it; otherwise inject the APIServerEndpoint endpoint defined in cluster status
apiServerEndpoint := config.Spec.JoinConfiguration.Discovery.BootstrapToken.APIServerEndpoint
if apiServerEndpoint == "" {
if !cluster.Spec.ControlPlaneEndpoint.IsValid() {
log.V(1).Info("Waiting for Cluster Controller to set Cluster.Spec.ControlPlaneEndpoint")
return ctrl.Result{RequeueAfter: 10 * time.Second}, nil
}
apiServerEndpoint = cluster.Spec.ControlPlaneEndpoint.String()
config.Spec.JoinConfiguration.Discovery.BootstrapToken.APIServerEndpoint = apiServerEndpoint
log.V(3).Info("Altering JoinConfiguration.Discovery.BootstrapToken.APIServerEndpoint", "apiServerEndpoint", apiServerEndpoint)
}
// if BootstrapToken already contains a token, respect it; otherwise create a new bootstrap token for the node to join
if config.Spec.JoinConfiguration.Discovery.BootstrapToken.Token == "" {
remoteClient, err := r.ClusterCache.GetClient(ctx, util.ObjectKey(cluster))
if err != nil {
return ctrl.Result{}, err
}
token, err := createToken(ctx, remoteClient, r.TokenTTL)
if err != nil {
return ctrl.Result{}, errors.Wrapf(err, "failed to create new bootstrap token")
}
config.Spec.JoinConfiguration.Discovery.BootstrapToken.Token = token
log.V(3).Info("Altering JoinConfiguration.Discovery.BootstrapToken.Token")
}

Please correct If I missed anything @chrischdi @fabriziopandini

Copy link
Member

Choose a reason for hiding this comment

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

I think we are ok

Discovery has two mutually exclusive options, FileDiscovery and TokenDiscovery.

If File discovery is set, TokenDiscovery must not be set
If File discovery is not set, TokenDiscovery is used, either with the values specified by the users or, if those value are missing, with defaults.

Considering this, there wont be any difference in behaviour between not specifying the object, and specifying empty bootstrapToken: in both cases you accept all the defaults for TokenDiscovery

// token is a token used to validate cluster information
// fetched from the control-plane.
// +required
// +optional
// +kubebuilder:validation:MinLength=1
// +kubebuilder:validation:MaxLength=512
Token string `json:"token"`
Token string `json:"token,omitempty"`

// apiServerEndpoint is an IP or domain name to the API server from which info will be fetched.
// +optional
Expand Down
4 changes: 2 additions & 2 deletions bootstrap/kubeadm/api/v1beta2/kubeadm_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -628,10 +628,10 @@ type Discovery struct {
type BootstrapTokenDiscovery struct {
// token is a token used to validate cluster information
// fetched from the control-plane.
// +required
// +optional
// +kubebuilder:validation:MinLength=1
// +kubebuilder:validation:MaxLength=512
Token string `json:"token"`
Token string `json:"token,omitempty"`

// apiServerEndpoint is an IP or domain name to the API server from which info will be fetched.
// +optional
Expand Down
4 changes: 0 additions & 4 deletions bootstrap/kubeadm/config/crd/bases/bootstrap.cluster.x-k8s.io_kubeadmconfigs.yaml
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 0 additions & 4 deletions bootstrap/kubeadm/config/crd/bases/bootstrap.cluster.x-k8s.io_kubeadmconfigtemplates.yaml
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 0 additions & 4 deletions ...rolplane/kubeadm/config/crd/bases/controlplane.cluster.x-k8s.io_kubeadmcontrolplanes.yaml
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 0 additions & 4 deletions .../kubeadm/config/crd/bases/controlplane.cluster.x-k8s.io_kubeadmcontrolplanetemplates.yaml
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.