- 
                Notifications
    You must be signed in to change notification settings 
- Fork 1.4k
✨ Add EncryptionAlgorithm to Kubeadmconfig #12859
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
base: main
Are you sure you want to change the base?
Changes from all commits
cd1a25d
              41c4d39
              eefb585
              f8dac7e
              418ff3d
              f0820c0
              a41ee45
              File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
|  | @@ -72,6 +72,23 @@ const ( | |
| KubeadmConfigDataSecretNotAvailableReason = clusterv1.NotAvailableReason | ||
| ) | ||
|  | ||
| // EncryptionAlgorithmType can define an asymmetric encryption algorithm type. | ||
| // +kubebuilder:validation:Enum=ECDSA-P256;ECDSA-P384;RSA-2048;RSA-3072;RSA-4096 | ||
| type EncryptionAlgorithmType string | ||
|  | ||
| const ( | ||
| // EncryptionAlgorithmECDSAP256 defines the ECDSA encryption algorithm type with curve P256. | ||
| EncryptionAlgorithmECDSAP256 EncryptionAlgorithmType = "ECDSA-P256" | ||
| // EncryptionAlgorithmECDSAP384 defines the ECDSA encryption algorithm type with curve P384. | ||
| EncryptionAlgorithmECDSAP384 EncryptionAlgorithmType = "ECDSA-P384" | ||
| // EncryptionAlgorithmRSA2048 defines the RSA encryption algorithm type with key size 2048 bits. | ||
| EncryptionAlgorithmRSA2048 EncryptionAlgorithmType = "RSA-2048" | ||
| // EncryptionAlgorithmRSA3072 defines the RSA encryption algorithm type with key size 3072 bits. | ||
| EncryptionAlgorithmRSA3072 EncryptionAlgorithmType = "RSA-3072" | ||
| // EncryptionAlgorithmRSA4096 defines the RSA encryption algorithm type with key size 4096 bits. | ||
| EncryptionAlgorithmRSA4096 EncryptionAlgorithmType = "RSA-4096" | ||
| ) | ||
|  | ||
| // InitConfiguration contains a list of elements that is specific "kubeadm init"-only runtime | ||
| // information. | ||
| // +kubebuilder:validation:MinProperties=1 | ||
|  | @@ -199,6 +216,13 @@ type ClusterConfiguration struct { | |
| // +kubebuilder:validation:Minimum=1 | ||
| // +kubebuilder:validation:Maximum=36500 | ||
| CACertificateValidityPeriodDays int32 `json:"caCertificateValidityPeriodDays,omitempty"` | ||
|  | ||
| // encryptionAlgorithm holds the type of asymmetric encryption algorithm used for keys and certificates. | ||
| // Can be one of "RSA-2048", "RSA-3072", "RSA-4096", "ECDSA-P256" or "ECDSA-P384". | ||
| // If not specified, Cluster API will use RSA-2048 as default. | ||
| // This field is only supported with Kubernetes v1.31 or above. | ||
| There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Did some testing and luckily noticed that ECDSA-P384 is only available on Kubernetes v1.34 or above All others are fine as they have been around since v1.31: https://kubernetes.io/docs/reference/config-api/kubeadm-config.v1beta4/ | ||
| // +optional | ||
| EncryptionAlgorithm EncryptionAlgorithmType `json:"encryptionAlgorithm,omitempty"` | ||
| There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's set this field to RSA-4096 in test/e2e/data/infrastructure-docker/main/clusterclass-quick-start-runtimesdk.yaml KubeadmControlPlaneTemplate for some free e2e test coverage | ||
| } | ||
|  | ||
| // IsDefined returns true if the ClusterConfiguration is defined. | ||
|  | ||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|  | @@ -30,6 +30,7 @@ import ( | |||||
| "k8s.io/client-go/rest" | ||||||
| "sigs.k8s.io/controller-runtime/pkg/client" | ||||||
|  | ||||||
| bootstrapv1 "sigs.k8s.io/cluster-api/api/bootstrap/kubeadm/v1beta2" | ||||||
| clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2" | ||||||
| "sigs.k8s.io/cluster-api/controllers/clustercache" | ||||||
| "sigs.k8s.io/cluster-api/util/cache" | ||||||
|  | @@ -43,7 +44,7 @@ type ManagementCluster interface { | |||||
|  | ||||||
| GetMachinesForCluster(ctx context.Context, cluster *clusterv1.Cluster, filters ...collections.Func) (collections.Machines, error) | ||||||
| GetMachinePoolsForCluster(ctx context.Context, cluster *clusterv1.Cluster) (*clusterv1.MachinePoolList, error) | ||||||
| GetWorkloadCluster(ctx context.Context, clusterKey client.ObjectKey) (WorkloadCluster, error) | ||||||
| GetWorkloadCluster(ctx context.Context, clusterKey client.ObjectKey, keyEncryptionAlgorithm bootstrapv1.EncryptionAlgorithmType) (WorkloadCluster, error) | ||||||
| } | ||||||
|  | ||||||
| // Management holds operations on the management cluster. | ||||||
|  | @@ -59,13 +60,14 @@ type Management struct { | |||||
|  | ||||||
| // ClientCertEntry is an Entry for the Cache that stores the client cert. | ||||||
| type ClientCertEntry struct { | ||||||
| Cluster client.ObjectKey | ||||||
| ClientCert *tls.Certificate | ||||||
| Cluster client.ObjectKey | ||||||
| ClientCert *tls.Certificate | ||||||
| EncryptionAlgorithm bootstrapv1.EncryptionAlgorithmType | ||||||
| There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. let's also add Cluster.UID to the struct and the cache key + set it below where we use ClientCertEntry Otherwise we would re-use certs even through cluster re-creations (was broken before this PR but it's an easy change so let's just include it here) | ||||||
| } | ||||||
|  | ||||||
| // Key returns the cache key of a ClientCertEntry. | ||||||
| func (r ClientCertEntry) Key() string { | ||||||
| return r.Cluster.String() | ||||||
| return fmt.Sprintf("%s/%s", r.Cluster.String(), r.EncryptionAlgorithm) | ||||||
| } | ||||||
|  | ||||||
| // RemoteClusterConnectionError represents a failure to connect to a remote cluster. | ||||||
|  | @@ -77,7 +79,7 @@ type RemoteClusterConnectionError struct { | |||||
| // Error satisfies the error interface. | ||||||
| func (e *RemoteClusterConnectionError) Error() string { return e.Name + ": " + e.Err.Error() } | ||||||
|  | ||||||
| // Unwrap satisfies the unwrap error inteface. | ||||||
| // Unwrap satisfies the unwrap error interface. | ||||||
| func (e *RemoteClusterConnectionError) Unwrap() error { return e.Err } | ||||||
|  | ||||||
| // Get implements client.Reader. | ||||||
|  | @@ -111,7 +113,7 @@ func (m *Management) GetMachinePoolsForCluster(ctx context.Context, cluster *clu | |||||
|  | ||||||
| // GetWorkloadCluster builds a cluster object. | ||||||
| // The cluster comes with an etcd client generator to connect to any etcd pod living on a managed machine. | ||||||
| func (m *Management) GetWorkloadCluster(ctx context.Context, clusterKey client.ObjectKey) (WorkloadCluster, error) { | ||||||
| func (m *Management) GetWorkloadCluster(ctx context.Context, clusterKey client.ObjectKey, keyEncryptionAlgorithm bootstrapv1.EncryptionAlgorithmType) (WorkloadCluster, error) { | ||||||
| // TODO(chuckha): Inject this dependency. | ||||||
| // TODO(chuckha): memoize this function. The workload client only exists as long as a reconciliation loop. | ||||||
| restConfig, err := m.ClusterCache.GetRESTConfig(ctx, clusterKey) | ||||||
|  | @@ -142,15 +144,15 @@ func (m *Management) GetWorkloadCluster(ctx context.Context, clusterKey client.O | |||||
| // Get client cert from cache if possible, otherwise generate it and add it to the cache. | ||||||
| // TODO: When we implement ClusterConfiguration.EncryptionAlgorithm we should add it to | ||||||
| // the ClientCertEntries and make it part of the key. | ||||||
| 
      Comment on lines
    
      145
     to 
      146
    
   There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 
        Suggested change
       
 This is done now | ||||||
| if entry, ok := m.ClientCertCache.Has(ClientCertEntry{Cluster: clusterKey}.Key()); ok { | ||||||
| if entry, ok := m.ClientCertCache.Has(ClientCertEntry{Cluster: clusterKey, EncryptionAlgorithm: keyEncryptionAlgorithm}.Key()); ok { | ||||||
| clientCert = *entry.ClientCert | ||||||
| } else { | ||||||
| // The client cert expires after 10 years, but that's okay as the cache has a TTL of 1 day. | ||||||
| clientCert, err = generateClientCert(crtData, keyData) | ||||||
| clientCert, err = generateClientCert(crtData, keyData, keyEncryptionAlgorithm) | ||||||
| if err != nil { | ||||||
| return nil, err | ||||||
| } | ||||||
| m.ClientCertCache.Add(ClientCertEntry{Cluster: clusterKey, ClientCert: &clientCert}) | ||||||
| m.ClientCertCache.Add(ClientCertEntry{Cluster: clusterKey, ClientCert: &clientCert, EncryptionAlgorithm: keyEncryptionAlgorithm}) | ||||||
| } | ||||||
| } else { | ||||||
| clientCert, err = m.getAPIServerEtcdClientCert(ctx, clusterKey) | ||||||
|  | ||||||
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.