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

Use URL from linodego create response for etcd-backup endpoint #218

Merged
merged 6 commits into from
Mar 28, 2024
Merged
Show file tree
Hide file tree
Changes from 4 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
2 changes: 1 addition & 1 deletion api/v1alpha1/linodeobjectstoragebucket_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ type LinodeObjectStorageBucketSpec struct {
// +kubebuilder:default=0
KeyGeneration *int `json:"keyGeneration,omitempty"`

// SecretType sets the type for the access-keys secret that will be generated by the controller.
// SecretType sets the type for the bucket-details secret that will be generated by the controller.
// +optional
// +kubebuilder:default=addons.cluster.x-k8s.io/resource-set
SecretType string `json:"secretType,omitempty"`
Expand Down
24 changes: 15 additions & 9 deletions cloud/scope/object_storage_bucket.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@
namespace: kube-system
stringData:
bucket_name: %s
bucket_region: %s
bucket_endpoint: %s
access_key_rw: %s
secret_key_rw: %s
access_key_ro: %s
Expand All @@ -44,7 +46,7 @@
PatchHelper *patch.Helper
}

const AccessKeyNameTemplate = "%s-access-keys"
const AccessKeyNameTemplate = "%s-bucket-details"
const NumAccessKeys = 2

func validateObjectStorageBucketScopeParams(params ObjectStorageBucketScopeParams) error {
Expand Down Expand Up @@ -115,7 +117,7 @@

// GenerateKeySecret returns a secret suitable for submission to the Kubernetes API.
// The secret is expected to contain keys for accessing the bucket, as well as owner and controller references.
func (s *ObjectStorageBucketScope) GenerateKeySecret(ctx context.Context, keys [NumAccessKeys]*linodego.ObjectStorageKey) (*corev1.Secret, error) {
func (s *ObjectStorageBucketScope) GenerateKeySecret(ctx context.Context, keys [NumAccessKeys]*linodego.ObjectStorageKey, bucket *linodego.ObjectStorageBucket) (*corev1.Secret, error) {
for _, key := range keys {
if key == nil {
return nil, errors.New("expected two non-nil object storage keys")
Expand All @@ -126,8 +128,10 @@
// If the secret is of ClusterResourceSet type, encapsulate real data in the outer data
if s.Bucket.Spec.SecretType == "addons.cluster.x-k8s.io/resource-set" {
secretStringData = map[string]string{
"access-keys-secret.yaml": fmt.Sprintf(bucketDataSecret,
s.Bucket.Name,
"bucket-details-secret.yaml": fmt.Sprintf(bucketDataSecret,
bucket.Label,
bucket.Cluster,
bucket.Hostname,

Check warning on line 134 in cloud/scope/object_storage_bucket.go

View check run for this annotation

Codecov / codecov/patch

cloud/scope/object_storage_bucket.go#L131-L134

Added lines #L131 - L134 were not covered by tests
keys[0].AccessKey,
keys[0].SecretKey,
keys[1].AccessKey,
Expand All @@ -136,11 +140,13 @@
}
} else {
secretStringData = map[string]string{
"bucket_name": s.Bucket.Name,
"access_key_rw": keys[0].AccessKey,
"secret_key_rw": keys[0].SecretKey,
"access_key_ro": keys[1].AccessKey,
"secret_key_ro": keys[1].SecretKey,
"bucket_name": bucket.Label,
"bucket_region": bucket.Cluster,
"bucket_endpoint": bucket.Hostname,
"access_key_rw": keys[0].AccessKey,
"secret_key_rw": keys[0].SecretKey,
"access_key_ro": keys[1].AccessKey,
"secret_key_ro": keys[1].SecretKey,
}
}
secret := &corev1.Secret{
Expand Down
6 changes: 3 additions & 3 deletions cloud/scope/object_storage_bucket_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,7 @@ func TestGenerateKeySecret(t *testing.T) {
Namespace: "test-namespace",
},
Status: infrav1alpha1.LinodeObjectStorageBucketStatus{
KeySecretName: ptr.To("test-bucket-access-keys"),
KeySecretName: ptr.To("test-bucket-bucket-details"),
},
},
keys: [NumAccessKeys]*linodego.ObjectStorageKey{
Expand Down Expand Up @@ -379,7 +379,7 @@ func TestGenerateKeySecret(t *testing.T) {
Namespace: "test-namespace",
},
Status: infrav1alpha1.LinodeObjectStorageBucketStatus{
KeySecretName: ptr.To("test-bucket-access-keys"),
KeySecretName: ptr.To("test-bucket-bucket-details"),
},
},
keys: [NumAccessKeys]*linodego.ObjectStorageKey{
Expand Down Expand Up @@ -463,7 +463,7 @@ func TestGenerateKeySecret(t *testing.T) {
Bucket: testcase.Bucket,
}

secret, err := objScope.GenerateKeySecret(context.Background(), testcase.keys)
secret, err := objScope.GenerateKeySecret(context.Background(), testcase.keys, &linodego.ObjectStorageBucket{})
if testcase.expectedErr != nil {
require.ErrorContains(t, err, testcase.expectedErr.Error())
return
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,8 @@ spec:
type: integer
secretType:
default: addons.cluster.x-k8s.io/resource-set
description: SecretType sets the type for the access-keys secret that
will be generated by the controller.
description: SecretType sets the type for the bucket-details secret
that will be generated by the controller.
type: string
required:
- cluster
Expand Down
2 changes: 1 addition & 1 deletion controller/linodeobjectstoragebucket_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ func (r *LinodeObjectStorageBucketReconciler) reconcileApply(ctx context.Context
}

if keys[0] != nil && keys[1] != nil {
secret, err := bScope.GenerateKeySecret(ctx, keys)
secret, err := bScope.GenerateKeySecret(ctx, keys, bucket)
if err != nil {
bScope.Logger.Error(err, "Failed to generate key secret")
r.setFailure(bScope, err)
Expand Down
72 changes: 39 additions & 33 deletions controller/linodeobjectstoragebucket_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,19 +49,21 @@ import (
)

type AccessKeySecret struct {
APIVersion string `yaml:"apiVersion"`
Kind string `yaml:"kind"`
APIVersion string `json:"apiVersion"`
Kind string `json:"kind"`
Metadata struct {
Name string `yaml:"name"`
Namespace string `yaml:"namespace"`
} `yaml:"metadata"`
Name string `json:"name"`
Namespace string `json:"namespace"`
} `json:"metadata"`
StringData struct {
Bucket_Name string `yaml:"bucket_name"`
Access_Key_RW string `yaml:"access_key_rw"`
Secret_Key_RW string `yaml:"secret_key_rw"`
Access_Key_RO string `yaml:"access_key_ro"`
Secret_Key_RO string `yaml:"secret_key_ro"`
} `yaml:"stringData"`
BucketName string `json:"bucket_name"`
BucketRegion string `json:"bucket_region"`
BucketEndpoint string `json:"bucket_endpoint"`
AccessKeyRW string `json:"access_key_rw"`
SecretKeyRW string `json:"secret_key_rw"`
AccessKeyRO string `json:"access_key_ro"`
SecretKeyRO string `json:"secret_key_ro"`
} `json:"stringData"`
}

func mockLinodeClientBuilder(m *mock.MockLinodeObjectStorageClient) scope.LinodeObjectStorageClientBuilder {
Expand Down Expand Up @@ -176,13 +178,15 @@ var _ = Describe("lifecycle", Ordered, Label("bucket", "lifecycle"), func() {
Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(&secret), &secret)).To(Succeed())
Expect(secret.Data).To(HaveLen(1))
var key AccessKeySecret
unMarshallingErr := yaml.Unmarshal(secret.Data["access-keys-secret.yaml"], &key)
unMarshallingErr := yaml.Unmarshal(secret.Data["bucket-details-secret.yaml"], &key)
Expect(unMarshallingErr).NotTo(HaveOccurred())
Expect(key.StringData.Bucket_Name).To(Equal("lifecycle"))
Expect(key.StringData.Access_Key_RW).To(Equal("key-0"))
Expect(key.StringData.Secret_Key_RW).To(Equal(""))
Expect(key.StringData.Access_Key_RO).To(Equal("key-1"))
Expect(key.StringData.Secret_Key_RO).To(Equal(""))
Expect(key.StringData.BucketName).To(Equal("lifecycle"))
Expect(key.StringData.BucketRegion).To(Equal("cluster"))
Expect(key.StringData.BucketEndpoint).To(Equal("hostname"))
Expect(key.StringData.AccessKeyRW).To(Equal("key-0"))
Expect(key.StringData.SecretKeyRW).To(Equal(""))
Expect(key.StringData.AccessKeyRO).To(Equal("key-1"))
Expect(key.StringData.SecretKeyRO).To(Equal(""))

By("recording the expected events")
Expect(<-recorder.Events).To(ContainSubstring("Object storage keys assigned"))
Expand All @@ -192,7 +196,7 @@ var _ = Describe("lifecycle", Ordered, Label("bucket", "lifecycle"), func() {
By("logging the expected messages")
logOutput := testLogs.String()
Expect(logOutput).To(ContainSubstring("Reconciling apply"))
Expect(logOutput).To(ContainSubstring("Secret lifecycle-access-keys was applied with new access keys"))
Expect(logOutput).To(ContainSubstring("Secret lifecycle-bucket-details was applied with new access keys"))
})

It("should ensure the bucket's secret exists", func(ctx SpecContext) {
Expand Down Expand Up @@ -232,13 +236,15 @@ var _ = Describe("lifecycle", Ordered, Label("bucket", "lifecycle"), func() {
Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(&secret), &secret)).To(Succeed())
Expect(secret.Data).To(HaveLen(1))
var key AccessKeySecret
unMarshallingErr := yaml.Unmarshal(secret.Data["access-keys-secret.yaml"], &key)
unMarshallingErr := yaml.Unmarshal(secret.Data["bucket-details-secret.yaml"], &key)
Expect(unMarshallingErr).NotTo(HaveOccurred())
Expect(key.StringData.Bucket_Name).To(Equal("lifecycle"))
Expect(key.StringData.Access_Key_RW).To(Equal("key-0"))
Expect(key.StringData.Secret_Key_RW).To(Equal(""))
Expect(key.StringData.Access_Key_RO).To(Equal("key-1"))
Expect(key.StringData.Secret_Key_RO).To(Equal(""))
Expect(key.StringData.BucketName).To(Equal("lifecycle"))
Expect(key.StringData.BucketRegion).To(Equal("cluster"))
Expect(key.StringData.BucketEndpoint).To(Equal("hostname"))
Expect(key.StringData.AccessKeyRW).To(Equal("key-0"))
Expect(key.StringData.SecretKeyRW).To(Equal(""))
Expect(key.StringData.AccessKeyRO).To(Equal("key-1"))
Expect(key.StringData.SecretKeyRO).To(Equal(""))

By("recording the expected events")
Expect(<-recorder.Events).To(ContainSubstring("Object storage keys retrieved"))
Expand All @@ -248,7 +254,7 @@ var _ = Describe("lifecycle", Ordered, Label("bucket", "lifecycle"), func() {
By("logging the expected messages")
logOutput := testLogs.String()
Expect(logOutput).To(ContainSubstring("Reconciling apply"))
Expect(logOutput).To(ContainSubstring("Secret lifecycle-access-keys was applied with new access keys"))
Expect(logOutput).To(ContainSubstring("Secret lifecycle-bucket-details was applied with new access keys"))
})

It("should rotate the bucket's keys", func(ctx SpecContext) {
Expand Down Expand Up @@ -301,7 +307,7 @@ var _ = Describe("lifecycle", Ordered, Label("bucket", "lifecycle"), func() {
By("logging the expected messages")
logOutput := testLogs.String()
Expect(logOutput).To(ContainSubstring("Reconciling apply"))
Expect(logOutput).To(ContainSubstring("Secret lifecycle-access-keys was applied with new access keys"))
Expect(logOutput).To(ContainSubstring("Secret lifecycle-bucket-details was applied with new access keys"))
})

It("should revoke the bucket's keys", func(ctx SpecContext) {
Expand Down Expand Up @@ -533,7 +539,7 @@ var _ = Describe("apply", Label("bucket", "apply"), func() {

obj.Spec.KeyGeneration = ptr.To(1)
obj.Status.LastKeyGeneration = obj.Spec.KeyGeneration
obj.Status.KeySecretName = ptr.To("mock-access-keys")
obj.Status.KeySecretName = ptr.To("mock-bucket-details")
obj.Status.AccessKeyRefs = []int{0, 1}

bScope := scope.ObjectStorageBucketScope{
Expand Down Expand Up @@ -563,12 +569,12 @@ var _ = Describe("apply", Label("bucket", "apply"), func() {
mockK8sClient := mock.NewMockk8sClient(mockCtrl)
mockK8sClient.EXPECT().
Get(gomock.Any(), gomock.Any(), gomock.Any()).
Return(apierrors.NewNotFound(schema.GroupResource{Resource: "Secret"}, "mock-access-keys")).
Return(apierrors.NewNotFound(schema.GroupResource{Resource: "Secret"}, "mock-bucket-details")).
Times(1)

obj.Spec.KeyGeneration = ptr.To(1)
obj.Status.LastKeyGeneration = obj.Spec.KeyGeneration
obj.Status.KeySecretName = ptr.To("mock-access-keys")
obj.Status.KeySecretName = ptr.To("mock-bucket-details")
obj.Status.AccessKeyRefs = []int{0, 1}

bScope := scope.ObjectStorageBucketScope{
Expand Down Expand Up @@ -600,7 +606,7 @@ var _ = Describe("apply", Label("bucket", "apply"), func() {
mockK8sClient := mock.NewMockk8sClient(mockCtrl)
mockK8sClient.EXPECT().
Get(gomock.Any(), gomock.Any(), gomock.Any()).
Return(apierrors.NewNotFound(schema.GroupResource{Resource: "Secret"}, "mock-access-keys")).
Return(apierrors.NewNotFound(schema.GroupResource{Resource: "Secret"}, "mock-bucket-details")).
Times(1)
mockK8sClient.EXPECT().
Scheme().
Expand All @@ -609,7 +615,7 @@ var _ = Describe("apply", Label("bucket", "apply"), func() {

obj.Spec.KeyGeneration = ptr.To(1)
obj.Status.LastKeyGeneration = obj.Spec.KeyGeneration
obj.Status.KeySecretName = ptr.To("mock-access-keys")
obj.Status.KeySecretName = ptr.To("mock-bucket-details")
obj.Status.AccessKeyRefs = []int{0, 1}

bScope := scope.ObjectStorageBucketScope{
Expand Down Expand Up @@ -646,15 +652,15 @@ var _ = Describe("apply", Label("bucket", "apply"), func() {
Times(1)
mockK8sClient.EXPECT().
Get(gomock.Any(), gomock.Any(), gomock.Any()).
Return(apierrors.NewNotFound(schema.GroupResource{Resource: "Secret"}, "mock-access-keys")).
Return(apierrors.NewNotFound(schema.GroupResource{Resource: "Secret"}, "mock-bucket-details")).
Times(1)
mockK8sClient.EXPECT().
Create(gomock.Any(), gomock.Any(), gomock.Any()).
Return(errors.New("secret creation error"))

obj.Spec.KeyGeneration = ptr.To(1)
obj.Status.LastKeyGeneration = obj.Spec.KeyGeneration
obj.Status.KeySecretName = ptr.To("mock-access-keys")
obj.Status.KeySecretName = ptr.To("mock-bucket-details")
obj.Status.AccessKeyRefs = []int{0, 1}

bScope := scope.ObjectStorageBucketScope{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ spec:
# TODO: This is over-truncated to account for the Kubernetes access key Secret
value: (trim((truncate(($run), `52`)), '-'))
- name: access_keys_secret
value: (join('-', [($bucket), 'access-keys']))
value: (join('-', [($bucket), 'bucket-details']))
template: true
steps:
- name: step-00
Expand Down Expand Up @@ -54,7 +54,7 @@ spec:
ref:
apiVersion: v1
kind: Secret
name: (join('-', [($namespace), 'backups-access-keys']))
name: (join('-', [($namespace), 'backups-bucket-details']))
- name: step-05
try:
- assert:
Expand Down
18 changes: 16 additions & 2 deletions templates/addons/etcd-backup-restore/etcd-backup-restore.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -34,16 +34,30 @@ data:
"accessKeyID": "$${AWS_ACCESS_KEY_ID}",
"secretAccessKey": "$${AWS_SECRET_ACCESS_KEY}",
"region": "$${AWS_REGION}",
"endpoint": "$${AWS_REGION}.linodeobjects.com",
"endpoint": "$${AWS_ENDPOINT#*$$BUCKET_NAME.}",
"sseCustomerKey": "$${AWS_SSE_CUSTOMER_KEY}"
}
EOF
cat /data/credentials-file
volumeMounts:
- name: data-volume
mountPath: /data
env:
- name: "AWS_REGION"
value: ${OBJ_BUCKET_REGION}
valueFrom:
secretKeyRef:
name: etcd-backup
key: "bucket_region"
- name: "BUCKET_NAME"
valueFrom:
secretKeyRef:
name: etcd-backup
key: "bucket_name"
- name: "AWS_ENDPOINT"
valueFrom:
secretKeyRef:
name: etcd-backup
key: "bucket_endpoint"
- name: "AWS_ACCESS_KEY_ID"
valueFrom:
secretKeyRef:
Expand Down
4 changes: 2 additions & 2 deletions templates/addons/etcd-backup-restore/linode-obj.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,13 @@ spec:
apiVersion: addons.cluster.x-k8s.io/v1beta1
kind: ClusterResourceSet
metadata:
name: ${CLUSTER_NAME}-etcd-backup-access-keys
name: ${CLUSTER_NAME}-etcd-backup-bucket-details
spec:
clusterSelector:
matchLabels:
etcd-backup: "true"
cluster: ${CLUSTER_NAME}
resources:
- kind: Secret
name: ${CLUSTER_NAME}-etcd-backup-access-keys
name: ${CLUSTER_NAME}-etcd-backup-bucket-details
strategy: ApplyOnce