Skip to content
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

✨ CABPK can now read file contents from a Secret #3083

Merged
merged 4 commits into from
May 26, 2020
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
6 changes: 6 additions & 0 deletions bootstrap/kubeadm/api/v1alpha2/conversion.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ func (src *KubeadmConfig) ConvertTo(dstRaw conversion.Hub) error {
dst.Status.DataSecretName = restored.Status.DataSecretName
dst.Spec.Verbosity = restored.Spec.Verbosity
dst.Spec.UseExperimentalRetryJoin = restored.Spec.UseExperimentalRetryJoin
dst.Spec.Files = restored.Spec.Files
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this is what we want? Otherwise we have to match files based on some property field to match array items?

Copy link
Member

Choose a reason for hiding this comment

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

We should only restore the files that have ContentFrom populated (non-nil)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is this because the restored version might be stale where conversion was possible?

Copy link
Member

Choose a reason for hiding this comment

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

Correct, in case a v1alpha2 controller modified some of the files (even though unlikely because bootstrap should only happen once), we need to make sure to restore those that have the new schema.

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 not sure I follow what this is doing... we're setting dst.Spec.Files to restored.Spec.Files and then appending some of the restored files to dst.Spec.Files? Isn't that going to result in duplicates?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice catch.

I think the first line dst.Spec.Files = restores.Spec.Files is leftover from my first revision, it should only be the loop. The intent is to look at restored files with contentFrom and append them. We shouldn’t append restored files with content set.

Will PR shortly


return nil
}
Expand Down Expand Up @@ -129,3 +130,8 @@ func Convert_v1alpha2_KubeadmConfigSpec_To_v1alpha3_KubeadmConfigSpec(in *Kubead
func Convert_v1alpha3_KubeadmConfigSpec_To_v1alpha2_KubeadmConfigSpec(in *kubeadmbootstrapv1alpha3.KubeadmConfigSpec, out *KubeadmConfigSpec, s apiconversion.Scope) error {
return autoConvert_v1alpha3_KubeadmConfigSpec_To_v1alpha2_KubeadmConfigSpec(in, out, s)
}

// Convert_v1alpha3_File_To_v1alpha2_File converts from the Hub version (v1alpha3) of the File to this version.
func Convert_v1alpha3_File_To_v1alpha2_File(in *kubeadmbootstrapv1alpha3.File, out *File, s apiconversion.Scope) error {
return autoConvert_v1alpha3_File_To_v1alpha2_File(in, out, s)
}
16 changes: 15 additions & 1 deletion bootstrap/kubeadm/api/v1alpha2/conversion_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,21 @@ func TestConvertKubeadmConfig(t *testing.T) {
ObjectMeta: metav1.ObjectMeta{
Name: "hub",
},
Spec: v1alpha3.KubeadmConfigSpec{},
Spec: v1alpha3.KubeadmConfigSpec{
Files: []v1alpha3.File{
{
ContentFrom: &v1alpha3.FileSource{
Secret: &v1alpha3.SecretFileSource{
Name: "foo",
Key: "bar",
},
},
},
{
Content: "baz",
},
},
},
Status: v1alpha3.KubeadmConfigStatus{
Ready: true,
DataSecretName: pointer.StringPtr("secret-data"),
Expand Down
40 changes: 28 additions & 12 deletions bootstrap/kubeadm/api/v1alpha2/zz_generated.conversion.go

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

27 changes: 26 additions & 1 deletion bootstrap/kubeadm/api/v1alpha3/kubeadmbootstrapconfig_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,32 @@ type File struct {
Encoding Encoding `json:"encoding,omitempty"`

// Content is the actual content of the file.
Content string `json:"content"`
// +optional
Content string `json:"content,omitempty"`
vincepri marked this conversation as resolved.
Show resolved Hide resolved

// ContentFrom is a referenced source of content to populate the file.
// +optional
ContentFrom *FileSource `json:"contentFrom,omitempty"`
vincepri marked this conversation as resolved.
Show resolved Hide resolved
}

// FileSource is a union of all possible external source types for file data.
// Only one field may be populated in any given instance. Developers adding new
// sources of data for target systems should add them here.
type FileSource struct {
// Secret represents a secret that should populate this file.
Secret *SecretFileSource `json:"secret"`
alexeldeib marked this conversation as resolved.
Show resolved Hide resolved
}

// Adapts a Secret into a FileSource.
//
// The contents of the target Secret's Data field will be presented
// as files using the keys in the Data field as the file names.
type SecretFileSource struct {
// Name of the secret in the KubeadmBootstrapConfig's namespace to use.
Name string `json:"name"`

// Key is the key in the secret's data map for this value.
Key string `json:"key"`
}

// User defines the input for a generated user in cloud-init.
Expand Down
157 changes: 111 additions & 46 deletions bootstrap/kubeadm/api/v1alpha3/kubeadmbootstrapconfig_types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,62 +17,127 @@ limitations under the License.
package v1alpha3

import (
. "github.com/onsi/ginkgo"
"testing"

. "github.com/onsi/gomega"

"golang.org/x/net/context"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
)

// These tests are written in BDD-style using Ginkgo framework. Refer to
// http://onsi.github.io/ginkgo to learn more.

var _ = Describe("KubeadmConfig", func() {
var (
key types.NamespacedName
created, fetched *KubeadmConfig
)

BeforeEach(func() {
// Add any setup steps that needs to be executed before each test
})

AfterEach(func() {
// Add any teardown steps that needs to be executed after each test
})

// Add Tests for OpenAPI validation (or additional CRD features) specified in
// your API definition.
// Avoid adding tests for vanilla CRUD operations because they would
// test Kubernetes API server, which isn't the goal here.
Context("Create API", func() {

It("should create an object successfully", func() {

key = types.NamespacedName{
Name: "foo",
Namespace: "default",
}
created = &KubeadmConfig{
func TestClusterValidate(t *testing.T) {
cases := map[string]struct {
in *KubeadmConfig
expectErr bool
}{
"valid content": {
in: &KubeadmConfig{
ObjectMeta: metav1.ObjectMeta{
Name: "baz",
Namespace: "default",
},
Spec: KubeadmConfigSpec{
Files: []File{
{
Content: "foo",
},
},
},
},
},
"valid contentFrom": {
in: &KubeadmConfig{
ObjectMeta: metav1.ObjectMeta{
Name: "baz",
Namespace: "default",
},
Spec: KubeadmConfigSpec{
Files: []File{
{
ContentFrom: &FileSource{
Secret: &SecretFileSource{
Name: "foo",
Key: "bar",
},
},
},
},
},
},
},
"invalid content and contentFrom": {
in: &KubeadmConfig{
ObjectMeta: metav1.ObjectMeta{
Name: "foo",
Name: "baz",
Namespace: "default",
},
Spec: KubeadmConfigSpec{
Files: []File{
{
ContentFrom: &FileSource{},
Content: "foo",
},
},
},
},
expectErr: true,
},
"invalid contentFrom without name": {
in: &KubeadmConfig{
ObjectMeta: metav1.ObjectMeta{
Name: "baz",
Namespace: "default",
},
Spec: KubeadmConfigSpec{
Files: []File{
{
ContentFrom: &FileSource{
Secret: &SecretFileSource{
Key: "bar",
},
},
Content: "foo",
},
},
},
},
expectErr: true,
},
"invalid contentFrom without key": {
in: &KubeadmConfig{
ObjectMeta: metav1.ObjectMeta{
Name: "baz",
Namespace: "default",
},
Spec: KubeadmConfigSpec{
Files: []File{
{
ContentFrom: &FileSource{
Secret: &SecretFileSource{
Name: "foo",
},
},
Content: "foo",
},
},
},
},
expectErr: true,
},
}

for name, tt := range cases {
t.Run(name, func(t *testing.T) {
g := NewWithT(t)
if tt.expectErr {
g.Expect(tt.in.ValidateCreate()).NotTo(Succeed())
g.Expect(tt.in.ValidateUpdate(nil)).NotTo(Succeed())
} else {
g.Expect(tt.in.ValidateCreate()).To(Succeed())
g.Expect(tt.in.ValidateUpdate(nil)).To(Succeed())
}

By("creating an API obj")
Expect(k8sClient.Create(context.TODO(), created)).To(Succeed())

fetched = &KubeadmConfig{}
Expect(k8sClient.Get(context.TODO(), key, fetched)).To(Succeed())
Expect(fetched).To(Equal(created))

By("deleting the created object")
Expect(k8sClient.Delete(context.TODO(), created)).To(Succeed())
Expect(k8sClient.Get(context.TODO(), key, created)).ToNot(Succeed())
})

})

})
}
}
Loading