✨ SSM (AWS Systems Manager Parameter Store) as a Secret Backend #1924

Promaethius · 2020-09-04T18:41:09Z

What this PR does / why we need it:
Adds an additional secret backend for environments which do not have access to AWS Secrets Manager. SSM is available as an EC2 feature in all possible scopes: https://aws.amazon.com/compliance/services-in-scope/

Additionally, the format of SSM API and CLI calls are similar to SecretsManager so this should be a relatively painless addition.

Which issue(s) this PR fixes:
Fixes #1900

k8s-ci-robot · 2020-09-04T18:41:17Z

Hi @Promaethius. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

api/v1alpha3/awscluster_types.go

controllers/awsmachine_controller.go

pkg/cloud/scope/managedcontrolplane.go

controllers/awsmachine_controller.go

api/v1alpha3/awscluster_types.go

pkg/cloud/services/ssm/helpers.go

api/v1alpha3/awscluster_types.go

Promaethius · 2020-09-08T18:53:34Z

@randomvariable struggling with the GenerateCloudInitMIMEDocument functionality since I moved it from being a generic function to a service specific function. Not enough experience with go/mock:

Unexpected call to *mock_services.MockSecretInterface.GenerateCloudInitMIMEDocument([test 1 ]) at /root/go/src/sigs.k8s.io/cluster-api-provider-aws/pkg/cloud/services/mock_services/secretsmanager_machine_interface_mock.go:85 because: there are no expected calls of the method "GenerateCloudInitMIMEDocument" for that receiver

Had to generalize the SecretsManagerInterface and the SSMInterface into SecretInterface to allow either backend in function arguments or return values. With the possibility of S3 for Ignition in the back of my skull, I'm trying to make it easier for the next secret backend integrator.

pkg/internal/splitbytes/helpers.go

randomvariable · 2020-09-09T12:34:19Z

@randomvariable struggling with the GenerateCloudInitMIMEDocument functionality since I moved it from being a generic function to a service specific function. Not enough experience with go/mock:

I'll take a look as to how best to unblock you after doing the v0.6.0 release. Also, apologies if I don't recognise you in Slack, but worth joining the channel in https://kubernetes.slack.com/archives/CD6U2V71N and we can pair, zoom etc...

randomvariable · 2020-09-09T12:35:09Z

/milestone v0.6.1

Promaethius · 2020-09-10T23:14:02Z

@randomvariable took some of your advice in half and broke the MIME generation out into an external package and unit test while leaving UserData generation Service specific.

randomvariable · 2020-09-11T12:42:39Z

/ok-to-test

randomvariable

some changes, but otherwise looking good I think

api/v1alpha3/awsmachine_types.go

api/v1alpha2/awsmachine_types.go

api/v1alpha3/awsmachine_types.go

controllers/awsmachine_controller.go

Promaethius · 2020-09-21T21:29:17Z

with #1858 in the same release, it's important to note that this PR would require it

Promaethius · 2020-09-22T18:02:50Z

/hold

Promaethius · 2020-10-03T05:58:56Z

/test pull-cluster-api-provider-aws-e2e

api/v1alpha3/awsmachine_webhook.go

Promaethius · 2020-10-05T14:23:23Z

/test pull-cluster-api-provider-aws-e2e

Promaethius · 2020-10-05T17:38:30Z

@randomvariable changes made, tests passing. Am I missing anything?

richardcase · 2020-10-13T18:14:49Z

cmd/clusterawsadm/cloudformation/bootstrap/cluster_api_node.go

-			"secretsmanager:GetSecretValue",
-		},
+func (t Template) secretPolicy(secureSecretsBackend infrav1.SecretBackend) iamv1.StatementEntry {
+	switch secureSecretsBackend {


This is very similar to cluster_api_controller.go above. Do you think it would be possible to consolidate into 1 place and add the additional actions for the controller policy?

Not essential though.

This kinda thing pops up a lot in the clusterctl code, I think it can be handled in a separate PR as a mini refactor?

richardcase · 2020-10-13T18:37:51Z

This seems ok to me, small comment about the policies but generally

/lgtm

randomvariable · 2020-10-14T10:32:23Z

One last git pull upstream --rebase to account for the addition of the machinepool webhook

This should do it.

---
apiVersion: admissionregistration.k8s.io/v1beta1
kind: MutatingWebhookConfiguration
metadata:
  creationTimestamp: null
  name: mutating-webhook-configuration
webhooks:
- clientConfig:
    caBundle: Cg==
    service:
      name: webhook-service
      namespace: system
      path: /mutate-infrastructure-cluster-x-k8s-io-v1alpha3-awscluster
  failurePolicy: Fail
  matchPolicy: Equivalent
  name: default.awscluster.infrastructure.cluster.x-k8s.io
  rules:
  - apiGroups:
    - infrastructure.cluster.x-k8s.io
    apiVersions:
    - v1alpha3
    operations:
    - CREATE
    - UPDATE
    resources:
    - awsclusters
  sideEffects: None
- clientConfig:
    caBundle: Cg==
    service:
      name: webhook-service
      namespace: system
      path: /mutate-infrastructure-cluster-x-k8s-io-v1alpha3-awsmachine
  failurePolicy: Fail
  name: mutation.awsmachine.infrastructure.cluster.x-k8s.io
  rules:
  - apiGroups:
    - infrastructure.cluster.x-k8s.io
    apiVersions:
    - v1alpha3
    operations:
    - CREATE
    - UPDATE
    resources:
    - awsmachines
  sideEffects: None
- clientConfig:
    caBundle: Cg==
    service:
      name: webhook-service
      namespace: system
      path: /mutate-infrastructure-cluster-x-k8s-io-v1alpha3-awsmachinepool
  failurePolicy: Fail
  matchPolicy: Equivalent
  name: default.awsmachinepool.infrastructure.cluster.x-k8s.io
  rules:
  - apiGroups:
    - infrastructure.cluster.x-k8s.io
    apiVersions:
    - v1alpha3
    operations:
    - CREATE
    - UPDATE
    resources:
    - awsmachinepools
  sideEffects: None
- clientConfig:
    caBundle: Cg==
    service:
      name: webhook-service
      namespace: system
      path: /mutate-infrastructure-cluster-x-k8s-io-v1alpha3-awsmanagedmachinepool
  failurePolicy: Fail
  matchPolicy: Equivalent
  name: default.awsmanagedmachinepool.infrastructure.cluster.x-k8s.io
  rules:
  - apiGroups:
    - infrastructure.cluster.x-k8s.io
    apiVersions:
    - v1alpha3
    operations:
    - CREATE
    - UPDATE
    resources:
    - awsmanagedmachinepools
  sideEffects: None

---
apiVersion: admissionregistration.k8s.io/v1beta1
kind: ValidatingWebhookConfiguration
metadata:
  creationTimestamp: null
  name: validating-webhook-configuration
webhooks:
- clientConfig:
    caBundle: Cg==
    service:
      name: webhook-service
      namespace: system
      path: /validate-infrastructure-cluster-x-k8s-io-v1alpha3-awscluster
  failurePolicy: Fail
  matchPolicy: Equivalent
  name: validation.awscluster.infrastructure.cluster.x-k8s.io
  rules:
  - apiGroups:
    - infrastructure.cluster.x-k8s.io
    apiVersions:
    - v1alpha3
    operations:
    - CREATE
    - UPDATE
    resources:
    - awsclusters
  sideEffects: None
- clientConfig:
    caBundle: Cg==
    service:
      name: webhook-service
      namespace: system
      path: /validate-infrastructure-cluster-x-k8s-io-v1alpha3-awsmachine
  failurePolicy: Fail
  matchPolicy: Equivalent
  name: validation.awsmachine.infrastructure.cluster.x-k8s.io
  rules:
  - apiGroups:
    - infrastructure.cluster.x-k8s.io
    apiVersions:
    - v1alpha3
    operations:
    - CREATE
    - UPDATE
    resources:
    - awsmachines
  sideEffects: None
- clientConfig:
    caBundle: Cg==
    service:
      name: webhook-service
      namespace: system
      path: /validate-infrastructure-cluster-x-k8s-io-v1alpha3-awsmachinetemplate
  failurePolicy: Fail
  matchPolicy: Equivalent
  name: validation.awsmachinetemplate.infrastructure.x-k8s.io
  rules:
  - apiGroups:
    - infrastructure.cluster.x-k8s.io
    apiVersions:
    - v1alpha3
    operations:
    - CREATE
    - UPDATE
    resources:
    - awsmachinetemplates
  sideEffects: None
- clientConfig:
    caBundle: Cg==
    service:
      name: webhook-service
      namespace: system
      path: /validate-infrastructure-cluster-x-k8s-io-v1alpha3-awsmachinepool
  failurePolicy: Fail
  matchPolicy: Equivalent
  name: validation.awsmachinepool.infrastructure.cluster.x-k8s.io
  rules:
  - apiGroups:
    - infrastructure.cluster.x-k8s.io
    apiVersions:
    - v1alpha3
    operations:
    - CREATE
    - UPDATE
    resources:
    - awsmachinepools
  sideEffects: None
- clientConfig:
    caBundle: Cg==
    service:
      name: webhook-service
      namespace: system
      path: /validate-infrastructure-cluster-x-k8s-io-v1alpha3-awsmanagedmachinepool
  failurePolicy: Fail
  matchPolicy: Equivalent
  name: validation.awsmanagedmachinepool.infrastructure.cluster.x-k8s.io
  rules:
  - apiGroups:
    - infrastructure.cluster.x-k8s.io
    apiVersions:
    - v1alpha3
    operations:
    - CREATE
    - UPDATE
    resources:
    - awsmanagedmachinepools
  sideEffects: None

randomvariable · 2020-10-14T15:04:57Z

@detiber asks:

Not sure if this was discussed previously, but is there a specific use case in mind for having this as an API field rather than a configuration option on the controller manager?
Is there a specific use case in mind where one would want to choose different backends for individual Clusters/Machines vs having it a global configuration for an instance of the infrastructure provider?

In retrospect, yes, maybe we could have done at the controller flag level. I think we should be working towards finding a better solution overall for v1alpha4, given the issues we've run into with cloud-init.

The one use case I did however have in mind for having it on the AWSMachineTemplate is for FlatCar Linux which doesn't support multi-part MIME docs, but where Ignition does natively support S3.

randomvariable · 2020-10-14T15:28:47Z

/lgtm
/approve

k8s-ci-robot · 2020-10-14T15:28:57Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: randomvariable

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [randomvariable]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Sep 4, 2020

k8s-ci-robot requested review from chuckha and rudoi September 4, 2020 18:41

k8s-ci-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Sep 4, 2020

Promaethius force-pushed the ssm branch from bbc2b9d to 9fb4614 Compare September 4, 2020 20:55

richardcase reviewed Sep 7, 2020

View reviewed changes

api/v1alpha3/awscluster_types.go Outdated Show resolved Hide resolved

api/v1alpha3/awscluster_types.go Outdated Show resolved Hide resolved

controllers/awsmachine_controller.go Outdated Show resolved Hide resolved

randomvariable suggested changes Sep 8, 2020

View reviewed changes

Promaethius requested a review from randomvariable September 8, 2020 18:49

randomvariable reviewed Sep 8, 2020

View reviewed changes

pkg/internal/splitbytes/helpers.go Outdated Show resolved Hide resolved

k8s-ci-robot added this to the v0.6.1 milestone Sep 9, 2020

Promaethius force-pushed the ssm branch 2 times, most recently from e1458e0 to 94189f8 Compare September 10, 2020 23:11

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Sep 11, 2020

randomvariable suggested changes Sep 11, 2020

View reviewed changes

randomvariable reviewed Sep 11, 2020

View reviewed changes

api/v1alpha3/awsmachine_types.go Outdated Show resolved Hide resolved

randomvariable reviewed Sep 11, 2020

View reviewed changes

controllers/awsmachine_controller.go Outdated Show resolved Hide resolved

randomvariable reviewed Sep 11, 2020

View reviewed changes

controllers/awsmachine_controller.go Outdated Show resolved Hide resolved

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 22, 2020

Promaethius force-pushed the ssm branch 2 times, most recently from 206e747 to e346938 Compare September 24, 2020 03:48

k8s-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Oct 3, 2020

Promaethius force-pushed the ssm branch from 2657ab6 to 43ea980 Compare October 3, 2020 01:36

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 3, 2020

randomvariable reviewed Oct 5, 2020

View reviewed changes

api/v1alpha3/awsmachine_webhook.go Outdated Show resolved Hide resolved

Promaethius force-pushed the ssm branch from 7b01c20 to 3061078 Compare October 5, 2020 14:22

richardcase reviewed Oct 13, 2020

View reviewed changes

k8s-ci-robot assigned richardcase Oct 13, 2020

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 13, 2020

randomvariable approved these changes Oct 14, 2020

View reviewed changes

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 14, 2020

randomvariable mentioned this pull request Oct 14, 2020

✨ SSM (AWS Systems Manager Parameter Store) as a Secret Backend #2036

Closed

Support for AWS SSM Parameter Store as a Secret Backend

6e4da05

Promaethius force-pushed the ssm branch from 3061078 to 6e4da05 Compare October 14, 2020 14:55

k8s-ci-robot removed lgtm "Looks good to me", indicates that a PR is ready to be merged. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Oct 14, 2020

k8s-ci-robot assigned randomvariable Oct 14, 2020

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 14, 2020

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 14, 2020

k8s-ci-robot merged commit a229052 into kubernetes-sigs:master Oct 14, 2020

Promaethius mentioned this pull request Oct 16, 2020

REQUEST: New membership for Promaethius kubernetes/org#2274

Closed

6 tasks

This was referenced Jan 18, 2024

Please ignore (created by mistake) kubernetes/cloud-provider-aws#832

Closed

Machine with cloud-init 23.3.0 or newer fails to join cluster #4745

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

✨ SSM (AWS Systems Manager Parameter Store) as a Secret Backend #1924

✨ SSM (AWS Systems Manager Parameter Store) as a Secret Backend #1924

Promaethius commented Sep 4, 2020 •

edited

Loading

k8s-ci-robot commented Sep 4, 2020

Promaethius commented Sep 8, 2020 •

edited

Loading

randomvariable commented Sep 9, 2020

randomvariable commented Sep 9, 2020

Promaethius commented Sep 10, 2020

randomvariable commented Sep 11, 2020

randomvariable left a comment

Promaethius commented Sep 21, 2020

Promaethius commented Sep 22, 2020

Promaethius commented Oct 3, 2020

Promaethius commented Oct 5, 2020

Promaethius commented Oct 5, 2020

richardcase Oct 13, 2020

Promaethius Oct 13, 2020

richardcase commented Oct 13, 2020

randomvariable commented Oct 14, 2020

randomvariable commented Oct 14, 2020

randomvariable commented Oct 14, 2020

k8s-ci-robot commented Oct 14, 2020

✨ SSM (AWS Systems Manager Parameter Store) as a Secret Backend #1924

✨ SSM (AWS Systems Manager Parameter Store) as a Secret Backend #1924

Conversation

Promaethius commented Sep 4, 2020 • edited Loading

k8s-ci-robot commented Sep 4, 2020

Promaethius commented Sep 8, 2020 • edited Loading

randomvariable commented Sep 9, 2020

randomvariable commented Sep 9, 2020

Promaethius commented Sep 10, 2020

randomvariable commented Sep 11, 2020

randomvariable left a comment

Choose a reason for hiding this comment

Promaethius commented Sep 21, 2020

Promaethius commented Sep 22, 2020

Promaethius commented Oct 3, 2020

Promaethius commented Oct 5, 2020

Promaethius commented Oct 5, 2020

richardcase Oct 13, 2020

Choose a reason for hiding this comment

Promaethius Oct 13, 2020

Choose a reason for hiding this comment

richardcase commented Oct 13, 2020

randomvariable commented Oct 14, 2020

randomvariable commented Oct 14, 2020

randomvariable commented Oct 14, 2020

k8s-ci-robot commented Oct 14, 2020

Promaethius commented Sep 4, 2020 •

edited

Loading

Promaethius commented Sep 8, 2020 •

edited

Loading