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.

Sign up for GitHub

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

Log in to AWS ECR automatically using the cluster-provided credentials. #147

Closed
wants to merge 15 commits into from
Closed

Log in to AWS ECR automatically using the cluster-provided credentials. #147

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
fd5a039
Log in to AWS ECR automatically using the cluster-provided credentials.
MartinEmrich Jun 1, 2021
b7e90af
Merge branch 'main' of https://github.com/fluxcd/image-reflector-cont…
MartinEmrich Jun 17, 2021
133d1e3
Merge branch 'main' into main
MartinEmrich Jun 24, 2021
a56624b
Enable AWS ECR login with flag instead of env variable
MartinEmrich Jun 24, 2021
dd9fbbf
removed (as of now) unused return values
MartinEmrich Jun 24, 2021
22f2317
Bump API version to v0.11.0
squaremo Jun 28, 2021
4b48cf1
Write changelog entry for v0.11.0
squaremo Jun 28, 2021
5c94f3d
Bump controller version used in config
squaremo Jun 28, 2021
4542bd0
Merge pull request #155 from fluxcd/release-v0.11.0
squaremo Jun 28, 2021
5da1152
Correct apiVersion of v1beta1 samples
squaremo Jul 14, 2021
c22a8d0
Merge pull request #158 from fluxcd/samples-to-v1beta1
squaremo Jul 14, 2021
a6e1be8
Log in to AWS ECR automatically using the cluster-provided credentials.
MartinEmrich Jun 1, 2021
801d106
Enable AWS ECR login with flag instead of env variable
MartinEmrich Jun 24, 2021
782d111
removed (as of now) unused return values
MartinEmrich Jun 24, 2021
4a461c1
Merge branch 'main' of github.com:MartinEmrich/image-reflector-contro…
MartinEmrich Jul 22, 2021
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
77 changes: 77 additions & 0 deletions controllers/imagerepository_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,14 @@ import (
"context"
"crypto/tls"
"crypto/x509"
"encoding/base64"
"encoding/json"
"errors"
"fmt"
"net/http"
"net/url"
"os"
"regexp"
"strings"
"time"

Expand All @@ -44,6 +47,10 @@ import (
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/predicate"

"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/aws/session"
"github.com/aws/aws-sdk-go/service/ecr"

"github.com/fluxcd/pkg/apis/meta"
"github.com/fluxcd/pkg/runtime/events"
"github.com/fluxcd/pkg/runtime/metrics"
Expand Down Expand Up @@ -177,6 +184,54 @@ func (r *ImageRepositoryReconciler) Reconcile(ctx context.Context, req ctrl.Requ
return ctrl.Result{RequeueAfter: when}, nil
}

func parseAwsImageURL(imageUrl string) (accountId, awsEcrRegion, awsPartition, ecrRepository, tag string, err error) {
err = nil
registryUrlPartRe := regexp.MustCompile("([0-9+]*).dkr.ecr.([^/.]*).(amazonaws.com[.cn]*)/([^:]+):?(.*)")
registryUrlParts := registryUrlPartRe.FindAllStringSubmatch(imageUrl, -1)
if len(registryUrlParts) < 1 {
err = errors.New("imageUrl does not match AWS elastic container registry URL pattern")
return
}
accountId = registryUrlParts[0][1]
awsEcrRegion = registryUrlParts[0][2]
awsPartition = registryUrlParts[0][3]
ecrRepository = registryUrlParts[0][4]
if len(registryUrlParts[0]) <= 4 {
tag = ""
return
}
tag = registryUrlParts[0][5]
return
}

// TODO: Still missing from Flux 1:
// Caching of tokens (one per account/region pair), this fetches a fresh token every time
// handling of expiry
// Back-Off in case of errors
// Possibly: special behaviour for non-global partitions (China, GovCloud)
func getAwsECRLoginAuth(accountId, awsEcrRegion string) (authConfig authn.AuthConfig, err error) {
accountIDs := []string{accountId}
ecrService := ecr.New(session.Must(session.NewSession(&aws.Config{Region: aws.String(awsEcrRegion)})))
ecrToken, err := ecrService.GetAuthorizationToken(&ecr.GetAuthorizationTokenInput{
RegistryIds: aws.StringSlice(accountIDs),
})
if err != nil {
return
}

token, err := base64.StdEncoding.DecodeString(*ecrToken.AuthorizationData[0].AuthorizationToken)
if err != nil {
return
}

tokenSplit := strings.Split(string(token), ":")
authConfig = authn.AuthConfig{
Username: tokenSplit[0],
Password: tokenSplit[1],
}
return
}

func (r *ImageRepositoryReconciler) scan(ctx context.Context, imageRepo *imagev1.ImageRepository, ref name.Reference) error {
timeout := imageRepo.GetTimeout()
ctx, cancel := context.WithTimeout(ctx, timeout)
Expand Down Expand Up @@ -210,6 +265,28 @@ func (r *ImageRepositoryReconciler) scan(ctx context.Context, imageRepo *imagev1
options = append(options, remote.WithAuth(auth))
}

if accountId, awsEcrRegion, _, _, _, err := parseAwsImageURL(imageRepo.Spec.Image); err == nil {
MartinEmrich marked this conversation as resolved.
Show resolved Hide resolved
if _, present := os.LookupEnv("USE_ECR"); present {
Copy link
Member

Choose a reason for hiding this comment

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

I would prefer this as a flag to the controller, rather than an environment variable. Really only because that's more in the spirit of controller-runtime with its config file support -- in most respects environment vars and flags work equally well.

I think it is appropriate for it to be set at the controller level, since it's up to the platform admin to grant global access to ambient authority, rather than for users to claim it. (If there is a reason to opt in per image repository, then fine, but I would still want the flag on the controller to explicitly grant access to everyone who wants it).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I'll try to find out how to access the command line and change the PR....

But I have no Idea what "instead of merging main into this branch, please rebase this branch on main" means...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@squaremo I guess I got the command-line option working. As for the rebase topic, I brushed up on Git theory, but I have no clue on how to execute it here on Github without damaging anything (Yes, it's my very first Github PR).

Copy link
Member

Choose a reason for hiding this comment

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

I have no clue on how to execute it here on Github without damaging anything (Yes, it's my very first Github PR).

First GitHub PR high five! ✋
I don't think GitHub has an in-app way to do this; it's something you would do on your local computer, after cloning your fork of this repo.

But I have no Idea what "instead of merging main into this branch, please rebase this branch on main" means...

Since you're working from a fork, you'll have to add this repo as a <remote>, first:

git remote add upstream https://github.com/fluxcd/image-reflector-controller

Also, it's worth working in a branch, and reserving main for keeping track of the upstream main branch:

git switch -c ecr-auto-creds
git push -u origin ecr-auto-creds

(The second command pushes the new branch to your forked repo, and -u sets it as the default place to push any further commits on that branch)

Then, rebasing on the main branch here can be done with

git pull upstream main --rebase

A lot of the time, this will succeed without any more work from you. If you get into trouble, you can git rebase --abort to take you back to the state before the rebase (or pull --rebase).

To push the branch back to GitHub, you'll need to use --force-with-lease, because you've replaced the commits in the branch:

git push --force-with-lease # it knows where to push to, because of `-u` above

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@squaremo Thanks! I followed your instructions, and all commands succeeded. But apparently it is not possible to change the source branch of a Github PR, so it's still from my "main" branch :(

logr.FromContext(ctx).Info("Logging in to AWS ECR for " + imageRepo.Spec.Image)

authConfig, err := getAwsECRLoginAuth(accountId, awsEcrRegion)
if err != nil {
imagev1.SetImageRepositoryReadiness(
imageRepo,
metav1.ConditionFalse,
meta.ReconciliationFailedReason,
err.Error(),
)
return err
}

auth := authn.FromConfig(authConfig)
options = append(options, remote.WithAuth(auth))
} else {
logr.FromContext(ctx).Info("AWS ECR authentication is not enabled, to enable, set USE_ECR environment variable")
Copy link
Member

Choose a reason for hiding this comment

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

Not sure if implementation details from another layer (set USE_ECR) should be mentioned here, it may be better (and less maintenance heavy) to simply refer to the feature.

}
}

if imageRepo.Spec.CertSecretRef != nil {
var certSecret corev1.Secret
if imageRepo.Spec.SecretRef != nil && imageRepo.Spec.SecretRef.Name == imageRepo.Spec.CertSecretRef.Name {
Expand Down
1 change: 1 addition & 0 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ replace github.com/fluxcd/image-reflector-controller/api => ./api

require (
github.com/Masterminds/semver/v3 v3.1.1
github.com/aws/aws-sdk-go v1.33.18
github.com/dgraph-io/badger/v3 v3.2103.0
github.com/fluxcd/image-reflector-controller/api v0.10.0
github.com/fluxcd/pkg/apis/meta v0.10.0
Expand Down
6 changes: 6 additions & 0 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,8 @@ github.com/armon/consul-api v0.0.0-20180202201655-eb2c6b5be1b6/go.mod h1:grANhF5
github.com/armon/go-metrics v0.0.0-20180917152333-f0300d1749da/go.mod h1:Q73ZrmVTwzkszR9V5SSuryQ31EELlFMUz1kKyl939pY=
github.com/armon/go-radix v0.0.0-20180808171621-7fddfc383310/go.mod h1:ufUuZ+zHj4x4TnLV4JWEpy2hxWSpsRywHrMgIH9cCH8=
github.com/asaskevich/govalidator v0.0.0-20190424111038-f61b66f89f4a/go.mod h1:lB+ZfQJz7igIIfQNfa7Ml4HSf2uFQQRzpGGRXenZAgY=
github.com/aws/aws-sdk-go v1.33.18 h1:Ccy1SV2SsgJU3rfrD+SOhQ0jvuzfrFuja/oKI86ruPw=
github.com/aws/aws-sdk-go v1.33.18/go.mod h1:5zCpMtNQVjRREroY7sYe8lOMRSxkhG6MZveU8YkpAk0=
github.com/beorn7/perks v0.0.0-20180321164747-3a771d992973/go.mod h1:Dwedo/Wpr24TaqPxmxbtue+5NUziq4I4S80YR8gNf3Q=
github.com/beorn7/perks v1.0.0/go.mod h1:KWe93zE9D1o94FZ5RNwFwVgaQK1VOXiVxmqh+CedLV8=
github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM=
Expand Down Expand Up @@ -167,6 +169,8 @@ github.com/go-openapi/spec v0.19.3/go.mod h1:FpwSN1ksY1eteniUU7X0N/BgJ7a4WvBFVA8
github.com/go-openapi/spec v0.19.5/go.mod h1:Hm2Jr4jv8G1ciIAo+frC/Ft+rR2kQDh8JHKHb3gWUSk=
github.com/go-openapi/swag v0.19.2/go.mod h1:POnQmlKehdgb5mhVOsnJFsivZCEZ/vjK9gh66Z9tfKk=
github.com/go-openapi/swag v0.19.5/go.mod h1:POnQmlKehdgb5mhVOsnJFsivZCEZ/vjK9gh66Z9tfKk=
github.com/go-sql-driver/mysql v1.5.0 h1:ozyZYNQW3x3HtqT1jira07DN2PArx2v7/mN66gGcHOs=
github.com/go-sql-driver/mysql v1.5.0/go.mod h1:DCzpHaOWr8IXmIStZouvnhqoel9Qv2LBy8hT2VhHyBg=
github.com/go-stack/stack v1.8.0/go.mod h1:v0f6uXyyMGvRgIKkXu+yp6POWl0qKG85gN/melR3HDY=
github.com/go-task/slim-sprig v0.0.0-20210107165309-348f09dbbbc0/go.mod h1:fyg7847qk6SyHyPtNmDHnmrv/HOrqktSC+C9fM+CJOE=
github.com/gogo/protobuf v1.1.1/go.mod h1:r8qH/GZQm5c6nD/R0oafs1akxWv10x8SbQlK7atdtwQ=
Expand Down Expand Up @@ -284,6 +288,8 @@ github.com/imdario/mergo v0.3.5/go.mod h1:2EnlNZ0deacrJVfApfmtdGgDfMuh/nq6Ok1EcJ
github.com/imdario/mergo v0.3.12 h1:b6R2BslTbIEToALKP7LxUvijTsNI9TAe80pLWN2g/HU=
github.com/imdario/mergo v0.3.12/go.mod h1:jmQim1M+e3UYxmgPu/WyfjB3N3VflVyUjjjwH0dnCYA=
github.com/inconshreveable/mousetrap v1.0.0/go.mod h1:PxqpIevigyE2G7u3NXJIT2ANytuPF1OarO4DADm73n8=
github.com/jmespath/go-jmespath v0.3.0 h1:OS12ieG61fsCg5+qLJ+SsW9NicxNkg3b25OyT2yCeUc=
github.com/jmespath/go-jmespath v0.3.0/go.mod h1:9QtRXoHjLGCJ5IBSaohpXITPlowMeeYCZ7fLUTSywik=
github.com/jessevdk/go-flags v1.4.0/go.mod h1:4FA24M0QyGHXBuZZK/XkWh8h0e1EYbRYJSGM75WSRxI=
github.com/joefitzgerald/rainbow-reporter v0.1.0/go.mod h1:481CNgqmVHQZzdIbN52CupLJyoVwB10FQ/IQlF1pdL8=
github.com/jonboulle/clockwork v0.1.0/go.mod h1:Ii8DK3G1RaLaWxj9trq07+26W01tbo22gdxWY5EU2bo=
Expand Down