Skip to content

CertSAN added as a patch json in clusterclass patches #372

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

Open
deepakm-ntnx wants to merge 9 commits into
base: main
Choose a base branch
from
Open

CertSAN added as a patch json in clusterclass patches #372

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
5749ce9
removed enable-hostpath-provisioner entry
deepakm-ntnx Jan 19, 2024
0e7f589
reverted changed for older version yamls in tests to keep version parity
deepakm-ntnx Jan 22, 2024
a0371ac
made certSAN into a patch without variable at this time
deepakm-ntnx Jan 24, 2024
702e470
removed midflight changes from another PR
deepakm-ntnx Jan 24, 2024
368500f
removed midflight changes from another PR
deepakm-ntnx Jan 24, 2024
3db0b9f
fixed the map in the patch
deepakm-ntnx Jan 25, 2024
b1717cd
added variable for certSAN
deepakm-ntnx Jan 26, 2024
b6556c9
added blackduck scan fix
deepakm-ntnx Jan 26, 2024
26cba45
reverted as BD scan is still failing
deepakm-ntnx Jan 26, 2024
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
16 changes: 12 additions & 4 deletions templates/cluster-template-clusterclass.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -345,6 +345,11 @@ spec:
sudo: ALL=(ALL) NOPASSWD:ALL
sshAuthorizedKeys:
- '{{ .sshKey }}'
- op: add
path: /spec/template/spec/kubeadmConfigSpec/clusterConfiguration/apiServer
valueFrom:
template: |
certSANs: [ {{ .apiServerSigningCertExtraCertSANs }} ]
Copy link
Contributor

@thunderboltsid thunderboltsid Jan 26, 2024

Choose a reason for hiding this comment

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

this should just be {{ .apiServerSigningCertExtraCertSANs }}

Copy link
Contributor Author

@deepakm-ntnx deepakm-ntnx Jan 26, 2024

Choose a reason for hiding this comment

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

could you please elaborate why it should be? it works current way

Comment on lines +348 to +352
Copy link
Contributor

@dkoshkin dkoshkin Jan 27, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

This is the limitation of inline patches and the difficulty of testing it :(
Having path: /spec/template/spec/kubeadmConfigSpec/clusterConfiguration/apiServer will remove all other options in apiServer both from templates and set by previously applied patches.

I would suggest something like this (this is already being tested in DKP in e2e tests). This initializes an empty array and then sets certSANs if apiServerSigningCertExtraCertSANs is not empty. Please also note this includes new definitions and jsonPatches as these are unrelated patches to /spec/template/spec/kubeadmConfigSpec/users.

  - definitions:
    - jsonPatches:
      - op: add
        path: /spec/template/spec/kubeadmConfigSpec/clusterConfiguration/apiServer/certSANs
        value: []
      selector:
        apiVersion: controlplane.cluster.x-k8s.io/v1beta1
        kind: KubeadmControlPlaneTemplate
        matchResources:
          controlPlane: true
    description: Initializes the API server extra sans.
    name: initializeKubeAPIServerExtraSans
  - definitions:
    - jsonPatches:
      - op: add
        path: /spec/template/spec/kubeadmConfigSpec/clusterConfiguration/apiServer/certSANs
        valueFrom:
          variable: apiServerSigningCertExtraCertSANs
      selector:
        apiVersion: controlplane.cluster.x-k8s.io/v1beta1
        kind: KubeadmControlPlaneTemplate
        matchResources:
          controlPlane: true
    description: Sets the extraSANs for a cluster
    enabledIf: '{{ if . apiServerSigningCertExtraCertSANs }}true{{ end }}'
    name: extraSANs

or simplified (untested):

  - definitions:
    - jsonPatches:
      - op: add
        path: /spec/template/spec/kubeadmConfigSpec/clusterConfiguration/apiServer/certSANs
        value: []
      - op: add
        path: /spec/template/spec/kubeadmConfigSpec/clusterConfiguration/apiServer/certSANs
        valueFrom:
          variable: apiServerSigningCertExtraCertSANs
      selector:
        apiVersion: controlplane.cluster.x-k8s.io/v1beta1
        kind: KubeadmControlPlaneTemplate
        matchResources:
          controlPlane: true
    description: Sets the extraSANs for a cluster
    enabledIf: '{{ if . apiServerSigningCertExtraCertSANs }}true{{ end }}'
    name: extraSANs

selector:
apiVersion: controlplane.cluster.x-k8s.io/v1beta1
kind: KubeadmControlPlaneTemplate
Expand Down Expand Up @@ -498,6 +503,13 @@ spec:
port:
type: integer
type: object
- name: apiServerSigningCertExtraCertSANs
required: true
schema:
openAPIV3Schema:
description: Set extra Subject Alternative Names (SANs) for the API Server
signing certificate.
type: string
Copy link
Contributor

@thunderboltsid thunderboltsid Jan 26, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

this should be a list of strings

Copy link
Contributor Author

@deepakm-ntnx deepakm-ntnx Jan 26, 2024

Choose a reason for hiding this comment

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

wanted to avoid any issue as mentioned in kubernetes-sigs/cluster-api#6245

Comment on lines +506 to +512
Copy link
Contributor

@dkoshkin dkoshkin Jan 27, 2024

Choose a reason for hiding this comment

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

And then you can make this an optional list.

Suggested change
- name: apiServerSigningCertExtraCertSANs
required: true
schema:
openAPIV3Schema:
description: Set extra Subject Alternative Names (SANs) for the API Server
signing certificate.
type: string
- name: apiServerSigningCertExtraCertSANs
schema:
openAPIV3Schema:
description: Set extra Subject Alternative Names (SANs) for the API Server
signing certificate.
type: array
items:
type: string

- name: prismCentralEndpoint
required: true
schema:
Expand Down Expand Up @@ -604,10 +616,6 @@ spec:
kubeadmConfigSpec:
clusterConfiguration:
apiServer:
certSANs:
- localhost
- 127.0.0.1
- 0.0.0.0
extraArgs:
cloud-provider: external
tls-cipher-suites: ${TLS_CIPHER_SUITES=TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384,TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384,TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256,TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256}
Expand Down
2 changes: 2 additions & 0 deletions templates/cluster-template-topology.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ spec:
systemDiskSize: ${NUTANIX_SYSTEMDISK_SIZE=40Gi}
vcpuSockets: ${NUTANIX_MACHINE_VCPU_SOCKET=2}
vcpusPerSocket: ${NUTANIX_MACHINE_VCPU_PER_SOCKET=1}
- name: apiServerSigningCertExtraCertSANs
value: localhost, 127.0.0.1, 0.0.0.0
Copy link
Contributor

@thunderboltsid thunderboltsid Jan 26, 2024

Choose a reason for hiding this comment

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

this should be a list of strings rather than a CSV string

version: ${KUBERNETES_VERSION}
workers:
machineDeployments:
Expand Down
11 changes: 11 additions & 0 deletions templates/clusterclass/clusterclass.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,11 @@ spec:
sudo: ALL=(ALL) NOPASSWD:ALL
sshAuthorizedKeys:
- '{{ .sshKey }}'
- op: add
path: /spec/template/spec/kubeadmConfigSpec/clusterConfiguration/apiServer
valueFrom:
template: |
certSANs: [ {{ .apiServerSigningCertExtraCertSANs }} ]
Copy link
Contributor

@thunderboltsid thunderboltsid Jan 26, 2024

Choose a reason for hiding this comment

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

this should just be {{ .apiServerSigningCertExtraCertSANs }}

- selector:
apiVersion: bootstrap.cluster.x-k8s.io/v1beta1
kind: KubeadmConfigTemplate
Expand Down Expand Up @@ -246,6 +251,12 @@ spec:
port:
type: integer
type: object
- name: apiServerSigningCertExtraCertSANs
required: true
schema:
openAPIV3Schema:
description: Set extra Subject Alternative Names (SANs) for the API Server signing certificate.
type: string
Copy link
Contributor

@thunderboltsid thunderboltsid Jan 26, 2024

Choose a reason for hiding this comment

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

this should be a list of strings

- name: prismCentralEndpoint
required: true
schema:
Expand Down
4 changes: 0 additions & 4 deletions templates/clusterclass/kcpt.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,6 @@ spec:
kubeadmConfigSpec:
clusterConfiguration:
apiServer:
certSANs:
- localhost
- 127.0.0.1
- 0.0.0.0
extraArgs:
cloud-provider: external
tls-cipher-suites: ${TLS_CIPHER_SUITES=TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384,TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384,TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256,TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256}
Expand Down
2 changes: 2 additions & 0 deletions templates/topology/cluster-with-topology.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,3 +55,5 @@ spec:
imageName: "${NUTANIX_MACHINE_TEMPLATE_IMAGE_NAME}"
clusterName: "${NUTANIX_PRISM_ELEMENT_CLUSTER_NAME}"
subnetName: "${NUTANIX_SUBNET_NAME}"
- name: apiServerSigningCertExtraCertSANs
value: "localhost, 127.0.0.1, 0.0.0.0"
25 changes: 12 additions & 13 deletions test/e2e/capx_quick_start_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ package e2e

import (
. "github.com/onsi/ginkgo/v2"
"k8s.io/utils/pointer"
capi_e2e "sigs.k8s.io/cluster-api/test/e2e"
)

Expand All @@ -36,18 +35,18 @@ var _ = Describe("When following the Cluster API quick-start", Label("quickstart
})
})

var _ = Describe("When following the Cluster API quick-start with ClusterClass", Label("quickstart", "capx-feature-test"), func() {
capi_e2e.QuickStartSpec(ctx, func() capi_e2e.QuickStartSpecInput {
return capi_e2e.QuickStartSpecInput{
E2EConfig: e2eConfig,
ClusterctlConfigPath: clusterctlConfigPath,
BootstrapClusterProxy: bootstrapClusterProxy,
ArtifactFolder: artifactFolder,
SkipCleanup: skipCleanup,
Flavor: pointer.String("topology"),
}
})
})
// var _ = Describe("When following the Cluster API quick-start with ClusterClass [PR-Informing]", func() {
// QuickStartSpec(ctx, func() QuickStartSpecInput {
// return QuickStartSpecInput{
// E2EConfig: e2eConfig,
// ClusterctlConfigPath: clusterctlConfigPath,
// BootstrapClusterProxy: bootstrapClusterProxy,
// ArtifactFolder: artifactFolder,
// SkipCleanup: skipCleanup,
// Flavor: pointer.String("topology"),
// }
// })
// })

// // NOTE: This test requires an IPv6 management cluster (can be configured via IP_FAMILY=IPv6).
// var _ = Describe("When following the Cluster API quick-start with IPv6 [IPv6] [PR-Informing]", func() {
Expand Down
16 changes: 12 additions & 4 deletions test/e2e/data/infrastructure-nutanix/v1beta1/clusterclass-e2e.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -345,6 +345,11 @@ spec:
sudo: ALL=(ALL) NOPASSWD:ALL
sshAuthorizedKeys:
- '{{ .sshKey }}'
- op: add
path: /spec/template/spec/kubeadmConfigSpec/clusterConfiguration/apiServer
valueFrom:
template: |
certSANs: [ {{ .apiServerSigningCertExtraCertSANs }} ]
selector:
apiVersion: controlplane.cluster.x-k8s.io/v1beta1
kind: KubeadmControlPlaneTemplate
Expand Down Expand Up @@ -498,6 +503,13 @@ spec:
port:
type: integer
type: object
- name: apiServerSigningCertExtraCertSANs
required: true
schema:
openAPIV3Schema:
description: Set extra Subject Alternative Names (SANs) for the API Server
signing certificate.
type: string
- name: prismCentralEndpoint
required: true
schema:
Expand Down Expand Up @@ -604,10 +616,6 @@ spec:
kubeadmConfigSpec:
clusterConfiguration:
apiServer:
certSANs:
- localhost
- 127.0.0.1
- 0.0.0.0
extraArgs:
cloud-provider: external
tls-cipher-suites: ${TLS_CIPHER_SUITES=TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384,TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384,TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256,TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256}
Expand Down