Skip to content

[WIP] ✨Add support for defining KubeadmConfig files by reference (ConfigMap & Secret) #1860

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

Closed
wants to merge 3 commits into from
Closed

[WIP] ✨Add support for defining KubeadmConfig files by reference (ConfigMap & Secret) #1860

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5c68a95
Add file references to KubeadmConfig
nstogner Dec 7, 2019
6e0cbdb
make generate
nstogner Dec 7, 2019
dff06dd
Remove gopkg.in/yaml.v2 which snuck into go.mod and run: go mod tidy
nstogner Dec 7, 2019
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
19 changes: 17 additions & 2 deletions bootstrap/kubeadm/api/v1alpha3/kubeadmbootstrapconfig_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ limitations under the License.
package v1alpha3

import (
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
kubeadmv1beta1 "sigs.k8s.io/cluster-api/bootstrap/kubeadm/types/v1beta1"
)
Expand Down Expand Up @@ -137,8 +138,22 @@ type File struct {
// +optional
Encoding Encoding `json:"encoding,omitempty"`

// Content is the actual content of the file.
Content string `json:"content"`
// Content is the actual content of the file. Mutually exclusive with ContentFrom.
// +optional
Content string `json:"content,omitempty"`

// ContentFrom indicates the content for the file should be retrieved from a referenced resource. Mutually exclusive with Content.
// +optional
ContentFrom *FileContentSource `json:"contentFrom,omitempty"`
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it make sense to move this up a level? Instead of requiring one secret or configmap per file, leverage the structure of secrets and configmaps to allow a user to import multiple files from a single configmap or secret?

Copy link
Contributor

Choose a reason for hiding this comment

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

That would mean we'd have files for inline, and filesFrom (?) for content from confimaps/secrets?

Copy link
Contributor

Choose a reason for hiding this comment

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

That would mean we'd have files for inline, and filesFrom (?) for content from confimaps/secrets?

Possibly... There is also the issue of the additional metadata that is included in the individual file types as well. It just feels quite a bit heavy weight to force one secret/configmap per file, when these resources are intended to provide multiple files by design.

Copy link
Contributor

Choose a reason for hiding this comment

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

Right, in light of this, I think we probably should spend some more time data modeling.

Copy link
Author

Choose a reason for hiding this comment

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

Perhaps it makes sense to copy the same pattern that pod volumes use:
https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.16/#configmapvolumesource-v1-core
Allow for specifying default file metadata (in this case owner, permissions, encoding) at the directory level (see defaultMode in the above example), and then allow for overriding those at the file level (see items[].mode in the above example).

Copy link
Author

Choose a reason for hiding this comment

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

Do the configmap & secret sources in Kubernetes offer anything like this?

Pod volumes are a slightly different use-case in that the unit of concern is a dir rather than a file. Also, the volume source and the volume mount location are specified separately.

Copy link
Member

Choose a reason for hiding this comment

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

It should be documented in the code how we are preventing two machines referencing the same ConfigMap/Secrets, because this will break the clusterctl move logic.

Copy link
Author

Choose a reason for hiding this comment

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

@fabriziopandini Noted.

Are we happy to move forward with the above data structure?

Copy link
Member

Choose a reason for hiding this comment

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

I'm ok with it.
FYI I'm trying to overcome the limitation of the single cluster on a parallel thread about clusterctl move

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm ok moving forward. Note, we have a fairly aggressive timeline for getting v1alpha3 done, so this may make it, or be deferred to v1alpha4 - just want to be up front about that.

}

// FileContentSource defines references to objects to source file content from.
type FileContentSource struct {
// Selects a key of a ConfigMap.
ConfigMapKeyRef *corev1.ConfigMapKeySelector `json:"configMapKeyRef,omitempty"`

// Selects a key of a Secret.
SecretKeyRef *corev1.SecretKeySelector `json:"secretKeyRef,omitempty"`
}

// User defines the input for a generated user in cloud-init.
Expand Down
35 changes: 34 additions & 1 deletion bootstrap/kubeadm/api/v1alpha3/zz_generated.deepcopy.go
View file
Open in desktop

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

12 changes: 10 additions & 2 deletions bootstrap/kubeadm/cloudinit/cloudinit.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,20 @@ type BaseUserData struct {
Header string
PreKubeadmCommands []string
PostKubeadmCommands []string
AdditionalFiles []bootstrapv1.File
WriteFiles []bootstrapv1.File
Files []File
Users []bootstrapv1.User
NTP *bootstrapv1.NTP
}

// File to be written to disk by cloud-init.
type File struct {
Path string
Owner string
Permissions string
Encoding bootstrapv1.Encoding
Content string
}

func generate(kind string, tpl string, data interface{}) ([]byte, error) {
tm := template.New(kind).Funcs(defaultTemplateFuncMap)
if _, err := tm.Parse(filesTemplate); err != nil {
Expand Down
32 changes: 6 additions & 26 deletions bootstrap/kubeadm/cloudinit/cloudinit_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,7 @@ import (
"bytes"
"testing"

infrav1 "sigs.k8s.io/cluster-api/bootstrap/kubeadm/api/v1alpha3"
"sigs.k8s.io/cluster-api/bootstrap/kubeadm/internal/cluster"
"sigs.k8s.io/cluster-api/util/certs"
bootstrapv1 "sigs.k8s.io/cluster-api/bootstrap/kubeadm/api/v1alpha3"
)

func TestNewInitControlPlaneAdditionalFileEncodings(t *testing.T) {
Expand All @@ -31,33 +29,24 @@ func TestNewInitControlPlaneAdditionalFileEncodings(t *testing.T) {
Header: "test",
PreKubeadmCommands: nil,
PostKubeadmCommands: nil,
AdditionalFiles: []infrav1.File{
Files: []File{
{
Path: "/tmp/my-path",
Encoding: infrav1.Base64,
Encoding: bootstrapv1.Base64,
Content: "aGk=",
},
{
Path: "/tmp/my-other-path",
Content: "hi",
},
},
WriteFiles: nil,
Users: nil,
NTP: nil,
Users: nil,
NTP: nil,
},
Certificates: cluster.Certificates{},
ClusterConfiguration: "my-cluster-config",
InitConfiguration: "my-init-config",
}

for _, certificate := range cpinput.Certificates {
certificate.KeyPair = &certs.KeyPair{
Cert: []byte("some certificate"),
Key: []byte("some key"),
}
}

out, err := NewInitControlPlane(cpinput)
if err != nil {
t.Fatal(err)
Expand All @@ -84,23 +73,14 @@ func TestNewInitControlPlaneCommands(t *testing.T) {
Header: "test",
PreKubeadmCommands: []string{`"echo $(date) ': hello world!'"`},
PostKubeadmCommands: []string{"echo $(date) ': hello world!'"},
AdditionalFiles: nil,
WriteFiles: nil,
Files: nil,
Users: nil,
NTP: nil,
},
Certificates: cluster.Certificates{},
ClusterConfiguration: "my-cluster-config",
InitConfiguration: "my-init-config",
}

for _, certificate := range cpinput.Certificates {
certificate.KeyPair = &certs.KeyPair{
Cert: []byte("some certificate"),
Key: []byte("some key"),
}
}

out, err := NewInitControlPlane(cpinput)
if err != nil {
t.Fatal(err)
Expand Down
9 changes: 1 addition & 8 deletions bootstrap/kubeadm/cloudinit/controlplane_init.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,9 @@ limitations under the License.

package cloudinit

import (
"sigs.k8s.io/cluster-api/bootstrap/kubeadm/internal/cluster"
)

const (
controlPlaneCloudInit = `{{.Header}}
{{template "files" .WriteFiles}}
{{template "files" .Files}}
- path: /tmp/kubeadm.yaml
owner: root:root
permissions: '0640'
Expand All @@ -43,7 +39,6 @@ runcmd:
// ControlPlaneInput defines the context to generate a controlplane instance user data.
type ControlPlaneInput struct {
BaseUserData
cluster.Certificates

ClusterConfiguration string
InitConfiguration string
Expand All @@ -52,8 +47,6 @@ type ControlPlaneInput struct {
// NewInitControlPlane returns the user data string to be used on a controlplane instance.
func NewInitControlPlane(input *ControlPlaneInput) ([]byte, error) {
input.Header = cloudConfigHeader
input.WriteFiles = input.Certificates.AsFiles()
input.WriteFiles = append(input.WriteFiles, input.AdditionalFiles...)
userData, err := generate("InitControlplane", controlPlaneCloudInit, input)
if err != nil {
return nil, err
Expand Down
6 changes: 1 addition & 5 deletions bootstrap/kubeadm/cloudinit/controlplane_join.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,11 @@ package cloudinit

import (
"github.com/pkg/errors"
"sigs.k8s.io/cluster-api/bootstrap/kubeadm/internal/cluster"
)

const (
controlPlaneJoinCloudInit = `{{.Header}}
{{template "files" .WriteFiles}}
{{template "files" .Files}}
- path: /tmp/kubeadm-controlplane-join-config.yaml
owner: root:root
permissions: '0640'
Expand All @@ -41,7 +40,6 @@ runcmd:
// ControlPlaneJoinInput defines context to generate controlplane instance user data for control plane node join.
type ControlPlaneJoinInput struct {
BaseUserData
cluster.Certificates

BootstrapToken string
JoinConfiguration string
Expand All @@ -51,8 +49,6 @@ type ControlPlaneJoinInput struct {
func NewJoinControlPlane(input *ControlPlaneJoinInput) ([]byte, error) {
input.Header = cloudConfigHeader
// TODO: Consider validating that the correct certificates exist. It is different for external/stacked etcd
input.WriteFiles = input.Certificates.AsFiles()
input.WriteFiles = append(input.WriteFiles, input.AdditionalFiles...)
userData, err := generate("JoinControlplane", controlPlaneJoinCloudInit, input)
if err != nil {
return nil, errors.Wrapf(err, "failed to generate user data for machine joining control plane")
Expand Down
3 changes: 1 addition & 2 deletions bootstrap/kubeadm/cloudinit/node.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ package cloudinit

const (
nodeCloudInit = `{{.Header}}
{{template "files" .WriteFiles}}
{{template "files" .Files}}
- path: /tmp/kubeadm-node.yaml
owner: root:root
permissions: '0640'
Expand All @@ -44,6 +44,5 @@ type NodeInput struct {
// NewNode returns the user data string to be used on a node instance.
func NewNode(input *NodeInput) ([]byte, error) {
input.Header = cloudConfigHeader
input.WriteFiles = append(input.WriteFiles, input.AdditionalFiles...)
return generate("Node", nodeCloudInit, input)
}
Loading