Skip to content

Commit 10bc7a3

Browse files
committed
Address review comment
- Provide AWS doc link - Check IsAWSECRURL in Object once - Check credential AWS access/secret key pair exists Signed-off-by: JenTing Hsiao <[email protected]>
1 parent fe1542d commit 10bc7a3

File tree

9 files changed

+22
-32
lines changed

9 files changed

+22
-32
lines changed

components/registry-credential/README.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,8 @@ kubectx [cluster-name]
1515

1616
```console
1717
kubectl create secret generic aws-iam-credential \
18-
--from-literal=access_key_id=<AWS_ACCESS_KEY> \
19-
--from-literal=secret_access_key=<AWS_SECRET_KEY>
18+
--from-literal=accessKeyId=<AWS_ACCESS_KEY> \
19+
--from-literal=secretAccessKey=<AWS_SECRET_KEY>
2020
```
2121

2222
### Prepare the configuration

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,8 @@ import (
2727
)
2828

2929
const (
30-
accessKeyIdName = "access_key_id"
31-
secretAccessKeyName = "secret_access_key"
30+
accessKeyIdName = "accessKeyId"
31+
secretAccessKeyName = "secretAccessKey"
3232
)
3333

3434
const (

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ func isAWSECRURL(URL string) bool {
3232

3333
// isPrivateAWSECRURL check if it's a private AWS ECR URL.
3434
// The private AWS ECR URL with the format aws_account_id.dkr.ecr.region.amazonaws.com.
35+
// Reference to https://docs.aws.amazon.com/AmazonECR/latest/userguide/Registries.html
3536
func isPrivateAWSECRURL(URL string) bool {
3637
u, err := url.Parse(URL)
3738
if err != nil {

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

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,6 @@ import (
1515
)
1616

1717
func configmap(ctx *common.RenderContext) ([]runtime.Object, error) {
18-
if !IsAWSECRURL(ctx) {
19-
return nil, nil
20-
}
21-
2218
privateRegistry := isPrivateAWSECRURL(ctx.Config.ContainerRegistry.External.URL)
2319
region := getAWSRegion(ctx.Config.ContainerRegistry.External.URL)
2420

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

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,6 @@ import (
1515
)
1616

1717
func cronjob(ctx *common.RenderContext) ([]runtime.Object, error) {
18-
if !IsAWSECRURL(ctx) {
19-
return nil, nil
20-
}
21-
2218
objectMeta := metav1.ObjectMeta{
2319
Name: Component,
2420
Namespace: ctx.Namespace,

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

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -10,15 +10,15 @@ import (
1010
"github.com/gitpod-io/gitpod/installer/pkg/common"
1111
)
1212

13-
var Objects = common.CompositeRenderFunc(
14-
configmap,
15-
role,
16-
rolebinding,
17-
cronjob,
18-
func(ctx *common.RenderContext) ([]runtime.Object, error) {
19-
if !IsAWSECRURL(ctx) {
20-
return nil, nil
21-
}
22-
return common.DefaultServiceAccount(Component)(ctx)
23-
},
24-
)
13+
func Objects(ctx *common.RenderContext) ([]runtime.Object, error) {
14+
if !IsAWSECRURL(ctx) {
15+
return nil, nil
16+
}
17+
return common.CompositeRenderFunc(
18+
configmap,
19+
role,
20+
rolebinding,
21+
cronjob,
22+
common.DefaultServiceAccount(Component),
23+
)(ctx)
24+
}

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

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,6 @@ import (
1313
)
1414

1515
func role(ctx *common.RenderContext) ([]runtime.Object, error) {
16-
if !IsAWSECRURL(ctx) {
17-
return nil, nil
18-
}
19-
2016
return []runtime.Object{
2117
&rbacv1.Role{
2218
TypeMeta: common.TypeMetaRole,

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

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,6 @@ import (
1313
)
1414

1515
func rolebinding(ctx *common.RenderContext) ([]runtime.Object, error) {
16-
if !IsAWSECRURL(ctx) {
17-
return nil, nil
18-
}
19-
2016
return []runtime.Object{
2117
&rbacv1.RoleBinding{
2218
TypeMeta: common.TypeMetaRoleBinding,

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,11 @@ func (v version) ClusterValidation(rcfg interface{}) cluster.ValidationChecks {
149149
if cfg.ContainerRegistry.External != nil {
150150
secretName := cfg.ContainerRegistry.External.Certificate.Name
151151
res = append(res, cluster.CheckSecret(secretName, cluster.CheckSecretRequiredData(".dockerconfigjson")))
152+
153+
if cfg.ContainerRegistry.External.Credential != nil {
154+
credSecretName := cfg.ContainerRegistry.External.Credential.Name
155+
res = append(res, cluster.CheckSecret(credSecretName, cluster.CheckSecretRequiredData("accessKeyId", "secretAccessKey")))
156+
}
152157
}
153158

154159
if cfg.ContainerRegistry.S3Storage != nil {

0 commit comments

Comments
 (0)