[refresh-credential] rotate the AWS ECR credential by schedule #15313

jenting · 2022-12-12T16:46:08Z

Description

Add a new component refresh-credential to rotate the AWS ECR authorization token by schedule.

The refresh-credential component is installed once the containerRegistry.external.url is an AWS ECR URL.

The private AWS ECR URL with the format aws_account_id.dkr.ecr.region.amazonaws.com.
The public AWS ECR URL with the format public.ecr.aws/<registry-alias>.

Remember to configure the

Create a Kubernetes secret with AWS access/secret key pair.

aws configure

# default path is ~/.aws/credentials
kubectl create secret aws-iam-credential -n default --from-file=</path/to/credentialsfile>

containerRegistry.inCluster = false to indicate to use external container registry.
containerRegistry.external.certificate indicates the secret name the AWS ECR authorization token write to.

containerRegistry.external.credentials which points to the AWS secret that with the AWS access/secret key pair.
Example yaml manifest:

containerRegistry:
  inCluster: false
  external:
    url: 012345678969.dkr.ecr.us-west-1.amazonaws.com
    certificate:
      kind: secret
      name: aws-ecr-credential
    credentials:
      kind: secret
      name: aws-iam-credential

The refresh-credential run as Kubernetes CronJob. The AWS ECR authorization token expires every 12 hours. The CronJob runs every 6 hours. Therefore, the on-caller have time to mitigate it once the ECR authorization token rotation fails.

We can add an alert rule when the job hits the backoffLimit. The trigger criteria is to query kube_job_status_failed{job_name=~"refresh-credential.*",reason="BackoffLimitExceeded"} > 0.
Reference to kube_job_status_failed.

https://www.loom.com/share/6d4b9dedc2df4a32bea87c12e3533230

Note
Since we haven't migrate/sync all the Gitpod component container image from GCP GCR to AWS ECR. Therefore, the secret with .dockerconfigjson requires to have both GCP GCR and AWS ECR authorization token because the image-builder push to AWS ECR, however the blobserve and registry-facade pull from GCP GCR.

Note
Because we set the Job restartPolicy = OnFailure, the pod will be terminated once the max backoff retry limit reaches. So we need to rely on the logging system to troubleshoot.

Warning
Please aware the service quotas.

The private AWS ECR service quotas.

The public AWS ECR service quotas.

Related Issue(s)

Fixes #12104

How to test

Try https://g704b4152c75da6e18a1605.workspace-preview.gitpod-io-dev.com/workspaces

Trigger a imagebuild
Check the base-image and workspace-image on the AWS ECR (us-west-1)

Release Notes

Support AWS ECR container registry

Documentation

None

Werft options:

/werft with-local-preview
If enabled this will build install/preview
/werft with-preview
/werft with-large-vm
/werft with-integration-tests=all
Valid options are all, workspace, webapp, ide, jetbrains, vscode, ssh

According to Kubernetes doc, a container using a Secret as a subPath volume mount will not receive Secret updates. Signed-off-by: JenTing Hsiao <[email protected]>

Signed-off-by: JenTing Hsiao <[email protected]>

werft-gitpod-dev-com · 2022-12-13T03:54:15Z

started the job as gitpod-build-jenting-aws-ecr-14914.68 because the annotations in the pull request description changed
(with .werft/ from main)

mrsimonemms

Looks broadly ok, but I've put a couple of questions/comments in there. Around on Slack if you want to chat about them.

install/installer/pkg/config/v1/config.go

install/installer/pkg/components/registry-credential/common.go

install/installer/pkg/components/registry-credential/cronjob.go

Pothulapati

Small nits, but looks good to me!

/hold just in case. Feel free to remove

install/installer/pkg/components/registry-credential/objects.go

Pothulapati · 2022-12-14T13:11:00Z

install/installer/pkg/components/image-builder-mk3/deployment.go

@@ -81,6 +81,7 @@ func deployment(ctx *common.RenderContext) ([]runtime.Object, error) {
 			VolumeSource: corev1.VolumeSource{
 				Secret: &corev1.SecretVolumeSource{
 					SecretName: secretName,
+					Items:      []corev1.KeyToPath{{Key: ".dockerconfigjson", Path: "pull-secret.json"}},


nit: Is pull-secret the right name? as we would also use it for pushes here. Could be tackled with https://github.com/gitpod-io/security/issues/89

The name is pull-secret.json now, ref code.
I updated the https://github.com/gitpod-io/security/issues/89 to track it.

- Provide AWS doc/code link - Check IsAWSECRURL in Object once - Check credential AWS access/secret key pair exists Signed-off-by: JenTing Hsiao <[email protected]>

jenting · 2022-12-14T14:15:51Z

@mrsimonemms PTAL 🙏

install/installer/pkg/config/v1/validation.go

mrsimonemms

Looks good, just one additional question

kylos101

Hi @jenting , wow, solid work!

Can you share an updated Loom video showing the test?

Also, I think we'll need follow-on PRs:

We should be using IRSA, instead of persisting client_id and client_secret as a Kubernetes secret
What do you think of renaming registry-credential to refresh-ecr-credential? Naming things is hard. 😉
What is the plan for creating an alert for when this job fails? If it attempts 10 times and still fails, how much time would that leave for us to react, before the cluster is unable to interact with the registry?

kylos101 · 2022-12-14T19:40:41Z

components/registry-credential/pkg/ecr/updater.go

+	accessKey := string(credSecret.Data[accessKeyIdName])
+	secretKey := string(credSecret.Data[secretAccessKeyName])


@jenting we should not be persisting an access and secret key to Kubernetes to interface with AWS. Instead, we should be leveraging an IAM role for a service account, aka IRSA.

In other words, we should avoid persisting long lived secrets to workspace clusters, and negotiate a temporary AWS role credential instead, to do the GetAuthorizationToken work.

After validating the token's signature, IAM exchanges the Kubernetes issued token for a temporary AWS role credential.

I think you'll need to use awsconfig.WithAssumeRoleCredentialOptions as an alternative to awsconfig.WithCredentialsProvider

Reference: https://docs.aws.amazon.com/eks/latest/userguide/associate-service-account-role.html

You can see an example from @mrzarquon here via ecr-helper service account.

@kylos101 I saw for S3 object storage, we use the shared config.

@Furisto Could you tell me how did you prepare the credential file you developed for the S3?

@Furisto Looks like you were using the shared config for development.

I update the doc, same as how the S3 load the credentials file.

@jenting I created a Kubernetes secret that contains the AWS shared config file (which gives access only to a specific bucket) and then mounted the file into the container. Then I reference that file when I am creating the client.. The file contains the credentials for a user that has no permissions except for the ones assigned by a bucket policy.

kylos101 · 2022-12-14T19:50:28Z

install/installer/pkg/config/v1/config.go

-	Certificate ObjectRef `json:"certificate" validate:"required"`
+	URL         string     `json:"url" validate:"required"`
+	Certificate ObjectRef  `json:"certificate" validate:"required"`
+	Credential  *ObjectRef `json:"credential,omitempty"`


I am skeptical we need this Credential field, please see my IRSA feedback for example.

components/BUILD.yaml

kylos101 · 2022-12-14T20:10:59Z

components/registry-credential/pkg/config/config.go

+
+	// CredentialSecret points to a Kubernetes secret which contains the credential to rotate
+	// the container registry credential .
+	CredentialSecret string `json:"credentialSecret"`


CredentialSecret should not be needed, I think, if we rely on IRSA.

kylos101 · 2022-12-14T20:21:42Z

components/registry-credential/pkg/ecr/updater.go

+	log.Infof("Secret %s/%s updated with new ECR credentials", cfg.SecretToUpdate, cfg.Namespace)
+}
+
+func newAWSConfig(region, accessKeyId, secretAccessKey, session string) (aws.Config, error) {


This will need to be changed, for example, to configure AWS given the service account associated with this new component.

kylos101 · 2022-12-14T20:23:02Z

install/installer/pkg/components/blobserve/configmap.go

@@ -101,7 +101,7 @@ func configmap(ctx *common.RenderContext) ([]runtime.Object, error) {
 				MaxSize:  MaxSizeBytes,
 			},
 		},
-		AuthCfg:            "/mnt/pull-secret.json",
+		AuthCfg:            "/mnt/pull-secret/pull-secret.json",


I forget, but, nesting the secret within a sub-directory was necessary, so that we can watch it for changes, right?

You're right 💯

FYR https://kubernetes.io/docs/concepts/configuration/secret/#:~:text=A%20container%20using%20a%20Secret%20as%20a%20subPath%20volume%20mount%20does%20not%20receive%20automated%20Secret%20updates

kylos101 · 2022-12-14T20:25:52Z

install/installer/pkg/components/registry-credential/configmap.go

+	credentialSecretName, err := credentialSecretName(ctx)
+	if err != nil {
+		return nil, err
+	}


Should be able to remove, given IRSA feedback

install/installer/pkg/components/registry-credential/cronjob.go

jenting · 2022-12-15T01:32:34Z

@kylos101

What do you think of renaming registry-credential to refresh-ecr-credential? Naming things is hard. 😉

We could rename it to refresh-credential because I don't want this component specific for ECR only.

What is the plan for creating an alert for when this job fails? If it attempts 10 times and still fails, how much time would that leave for us to react, before the cluster is unable to interact with the registry?

According to the Kubernetes doc, 10-time retry means the alert triggered after 10s+20s+40s+80s+160s+320s+360s+360s+360s+360s=34.5minutes.
Given the cronjob runs every 6 hours and the authorization token is valid for 12 hours. We have at least 5 hours to react.

jenting · 2022-12-15T04:09:02Z

We should be using IRSA, instead of persisting client_id and client_secret as a Kubernetes secret

I talked with @kylos101, and Kyle said since S3 requires the same secret, don't worry about my feedback for IRSA.
Therefore, I will change this PR following how S3 loads the credential files.

Correct me if I am wrong, but if I remember correctly, the IRSA (the AWs STS assume-role) is workable when the Kubernetes cluster lands on the AWS EC2 because the STS assume-role relies on the AWS EC2 information to grant the temporary credential.
We could change how we get an AWS credential file in the follow-up PR when required.

Signed-off-by: JenTing Hsiao <[email protected]>

When using concurrentPolicy=Replace and the job failed but haven't reach the backoff limit, the new job will replace the original one if the schedule time is less than the sum of the backoff time. It causes a problem that the job alert `kube_job_status_failed{job_name=~"refresh-credential.*",reason="BackoffLimitExceeded"}` can't be fired. Signed-off-by: JenTing Hsiao <[email protected]>

jenting · 2022-12-15T08:47:55Z

/unhold

All the comments are addressed. Thanks all the reviewers' comments.

We could have follow up PRs

if IRSA is required
if we want to separate the AWS IAM policy for the image-builder and registry-facade/blobserve.

roboquat added release-note-none do-not-merge/work-in-progress size/XXL labels Dec 12, 2022

jenting added 6 commits December 12, 2022 16:46

installer: update volume mount when Secret updates

db80e26

According to Kubernetes doc, a container using a Secret as a subPath volume mount will not receive Secret updates. Signed-off-by: JenTing Hsiao <[email protected]>

installer: add registry-credential component

4762c65

Signed-off-by: JenTing Hsiao <[email protected]>

A new component: registry-credential

101469b

Signed-off-by: JenTing Hsiao <[email protected]>

installer: use container image from leeway built

42c5372

Signed-off-by: JenTing Hsiao <[email protected]>

registry-credential: load from config file

8c15093

Signed-off-by: JenTing Hsiao <[email protected]>

installer: update according to load from config

2f7ebf9

Signed-off-by: JenTing Hsiao <[email protected]>

jenting force-pushed the jenting/aws-ecr-14914 branch from 9481d86 to 0c0918c Compare December 12, 2022 16:49

kylos101 mentioned this pull request Dec 12, 2022

Rotate container registry secret for Amazon's Elastic Container Registry (ECR) #12104

Closed

[installer]: add registry-credential as leeway dependency (#15306)

3ebe4f9

jenting force-pushed the jenting/aws-ecr-14914 branch from fd1920e to 2993448 Compare December 13, 2022 01:18

roboquat added release-note and removed release-note-none labels Dec 13, 2022

jenting marked this pull request as ready for review December 13, 2022 04:19

jenting requested review from a team December 13, 2022 04:19

roboquat removed the do-not-merge/work-in-progress label Dec 13, 2022

github-actions bot added team: SID team: IDE team: workspace Issue belongs to the Workspace team labels Dec 13, 2022

jenting marked this pull request as draft December 13, 2022 04:23

roboquat added the do-not-merge/work-in-progress label Dec 13, 2022

jenting force-pushed the jenting/aws-ecr-14914 branch 4 times, most recently from c962856 to 37a3115 Compare December 13, 2022 08:12

jenting force-pushed the jenting/aws-ecr-14914 branch from ee58365 to fe1542d Compare December 14, 2022 11:37

mrsimonemms suggested changes Dec 14, 2022

View reviewed changes

install/installer/pkg/config/v1/config.go Show resolved Hide resolved

install/installer/pkg/components/registry-credential/common.go Outdated Show resolved Hide resolved

install/installer/pkg/components/registry-credential/cronjob.go Outdated Show resolved Hide resolved

Pothulapati approved these changes Dec 14, 2022

View reviewed changes

roboquat added the do-not-merge/hold label Dec 14, 2022

Address review comment

ae4f39d

- Provide AWS doc/code link - Check IsAWSECRURL in Object once - Check credential AWS access/secret key pair exists Signed-off-by: JenTing Hsiao <[email protected]>

jenting force-pushed the jenting/aws-ecr-14914 branch from 10bc7a3 to ae4f39d Compare December 14, 2022 13:52

jenting requested a review from mrsimonemms December 14, 2022 14:15

mrsimonemms reviewed Dec 14, 2022

View reviewed changes

install/installer/pkg/config/v1/validation.go Outdated Show resolved Hide resolved

mrsimonemms reviewed Dec 14, 2022

View reviewed changes

mrsimonemms self-requested a review December 14, 2022 14:34

kylos101 reviewed Dec 14, 2022

View reviewed changes

jenting force-pushed the jenting/aws-ecr-14914 branch from 3998706 to 796b970 Compare December 15, 2022 03:56

jenting changed the title ~~[registry-credential] rotate the AWS ECR credential by schedule~~ [refresh-credential] rotate the AWS ECR credential by schedule Dec 15, 2022

jenting force-pushed the jenting/aws-ecr-14914 branch 2 times, most recently from d4f8cbe to 9d61e17 Compare December 15, 2022 05:15

jenting added 2 commits December 15, 2022 05:26

Rename from registry-credential to refresh-credential

397b18d

Signed-off-by: JenTing Hsiao <[email protected]>

Check secret key credentials

c09d2f9

Signed-off-by: JenTing Hsiao <[email protected]>

jenting force-pushed the jenting/aws-ecr-14914 branch 2 times, most recently from 2697c6a to c09d2f9 Compare December 15, 2022 06:46

mrsimonemms approved these changes Dec 15, 2022

View reviewed changes

roboquat removed the do-not-merge/hold label Dec 15, 2022

roboquat merged commit e7233ec into main Dec 15, 2022

roboquat deleted the jenting/aws-ecr-14914 branch December 15, 2022 08:48

roboquat added deployed: IDE IDE change is running in production deployed: workspace Workspace team change is running in production labels Dec 15, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[refresh-credential] rotate the AWS ECR credential by schedule #15313

[refresh-credential] rotate the AWS ECR credential by schedule #15313

jenting commented Dec 12, 2022 •

edited

Loading

werft-gitpod-dev-com bot commented Dec 13, 2022

mrsimonemms left a comment

Pothulapati left a comment

Pothulapati Dec 14, 2022

jenting Dec 14, 2022 •

edited

Loading

jenting commented Dec 14, 2022 •

edited by werft-gitpod-dev-com bot

Loading

mrsimonemms left a comment

kylos101 left a comment

kylos101 Dec 14, 2022

kylos101 Dec 14, 2022

kylos101 Dec 14, 2022

kylos101 Dec 14, 2022

kylos101 Dec 14, 2022

jenting Dec 15, 2022

jenting Dec 15, 2022

jenting Dec 15, 2022

Furisto Dec 15, 2022 •

edited

Loading

kylos101 Dec 14, 2022

kylos101 Dec 14, 2022

kylos101 Dec 14, 2022

kylos101 Dec 14, 2022

jenting Dec 15, 2022

jenting Dec 15, 2022

kylos101 Dec 14, 2022

jenting commented Dec 15, 2022 •

edited by werft-gitpod-dev-com bot

Loading

jenting commented Dec 15, 2022

jenting commented Dec 15, 2022

		accessKey := string(credSecret.Data[accessKeyIdName])
		secretKey := string(credSecret.Data[secretAccessKeyName])

[refresh-credential] rotate the AWS ECR credential by schedule #15313

[refresh-credential] rotate the AWS ECR credential by schedule #15313

Conversation

jenting commented Dec 12, 2022 • edited Loading

Description

Related Issue(s)

How to test

Release Notes

Documentation

Werft options:

werft-gitpod-dev-com bot commented Dec 13, 2022

mrsimonemms left a comment

Choose a reason for hiding this comment

Pothulapati left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jenting Dec 14, 2022 • edited Loading

Choose a reason for hiding this comment

jenting commented Dec 14, 2022 • edited by werft-gitpod-dev-com bot Loading

mrsimonemms left a comment

Choose a reason for hiding this comment

kylos101 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Furisto Dec 15, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jenting commented Dec 15, 2022 • edited by werft-gitpod-dev-com bot Loading

jenting commented Dec 15, 2022

jenting commented Dec 15, 2022

jenting commented Dec 12, 2022 •

edited

Loading

jenting Dec 14, 2022 •

edited

Loading

jenting commented Dec 14, 2022 •

edited by werft-gitpod-dev-com bot

Loading

Furisto Dec 15, 2022 •

edited

Loading

jenting commented Dec 15, 2022 •

edited by werft-gitpod-dev-com bot

Loading