Elaborate on how to enable autologin for ECR #193

kingdonb · 2021-10-31T15:24:01Z

There is not enough context in the documentation for --aws-autologin-for-ecr to guarantee that a new user who finds this section of the doc will be able to figure out where or how to add this flag to the image-reflector-controller.

Provide an example patches directive and be sure to say more explicitly which controller accepts this flag. (It is not at all obvious which controller owns the documentation at this URL, even though it has been pulled from this repo, there is no breadcrumb at all or anything else which says "image reflector controller")

https://fluxcd.io/docs/components/image/imagerepositories/#ecr-and-eks

…d): elaborate on how to enable autologin for ECR fluxcd/flux#1619 fluxcd/flux2#107 fluxcd#11 fluxcd#139 fluxcd#147 fluxcd#174 fluxcd#179 fluxcd#180 fluxcd/website#601 fluxcd#193 fluxcd/website#611 https://fluxcd.io/docs/guides/image-update/#aws-elastic-container-registry Signed-off-by: lloydchang <lloydchang@gmail.com>

…d): elaborate on how to enable autologin for ECR fluxcd/flux#1619 fluxcd/flux2#107 fluxcd#11 fluxcd#139 fluxcd#147 fluxcd#174 fluxcd#179 fluxcd#180 fluxcd/website#601 fluxcd#193 fluxcd/website#611 https://fluxcd.io/docs/guides/image-update/#aws-elastic-container-registry Signed-off-by: lloydchang <lloydchang@gmail.com> Signed-off-by: Kingdon Barrett <kingdon@weave.works>

kingdonb · 2021-10-31T19:22:08Z

I incorporated some additional feedback from @lloydchang based on kingdonb#1

This review also spotted and fixed some typos, not directly in the path of this change but in adjacent sections, which I think are close enough to include here. Thanks!

docs/spec/v1beta1/imagerepositories.md

relu

Looks pretty clear to me 💯

stefanprodan · 2021-11-02T11:47:52Z

docs/spec/v1beta1/imagerepositories.md

+
+Alternatively, the advice under [Other platforms][other platforms] below will also work for ECR.
+
+> You need to upgrade to Flux version 2 release [v0.19][Flux v0.19.0] that contains the image-reflector-controller release [v0.13.0][image-reflector-controller v0.13.0].


API specs usually don't contain changelogs, what's the reason we add these here?

As an experimental feature, I wanted to call out the release version that these docs applied to so that it is clear enough what needs to be updated when changes inevitably happen. (Also, it was a suggestion from @lloydchang)

I suppose the right place for this documentation might actually be here. This section should be updated anyway since it's likely a lot of folks will prefer this method of authentication over the CronJob approach.

squaremo · 2021-11-02T13:08:05Z

(It is not at all obvious which controller owns the documentation at this URL)

That's because it's API documentation -- no controller owns this documentation. I hesitated to refer to the flag here at all, because advice on how to run the controller really belongs elsewhere (but an elsewhere that doesn't exist, at present).

squaremo · 2021-11-02T13:10:10Z

Please keep the commit message subject to 50 characters -- see https://github.com/fluxcd/.github/blob/main/CONTRIBUTING.md#format-of-the-commit-message

stefanprodan · 2021-11-02T13:12:31Z

I think the patch example should be in the guide not here in the API docs.

kingdonb · 2021-11-02T13:28:32Z

I'll implement these changes and come back, thank you

darkowlzz · 2021-11-03T13:27:40Z

docs/spec/v1beta1/imagerepositories.md

@@ -190,7 +221,7 @@ type ImageRepositoryStatus struct {
 	// +optional
 	ObservedGeneration int64 `json:"observedGeneration,omitempty"`

-	// CannonicalName is the name of the image repository with all the
+	// CanonicalImageName is the name of the image repository with all the


This section is taken from https://github.com/fluxcd/image-reflector-controller/blob/main/api/v1beta1/imagerepository_types.go#L99 which also has this incorrect name. Can you update that as well and run make manifests to update the same in the CRD spec in config/crd/bases/image.toolkit.fluxcd.io_imagerepositories.yaml?

It looks like this has been taken care of. I found another CannonicalName in the docs and updated it there 👍

solms · 2021-11-30T13:18:08Z

Not sure how much value this adds, but just to let you know that I am a (new) Flux user who spent way too much time trying to figure out how to use the --aws-autologin-for-ecr, and this PR finally cleared it up and sorted me out :)

So +1 to new users being lost in this section and thanks @kingdonb for putting this out there!

Signed-off-by: Kingdon Barrett <kingdon@weave.works>

…d): elaborate on how to enable autologin for ECR fluxcd/flux#1619 fluxcd/flux2#107 fluxcd#11 fluxcd#139 fluxcd#147 fluxcd#174 fluxcd#179 fluxcd#180 fluxcd/website#601 fluxcd#193 fluxcd/website#611 https://fluxcd.io/docs/guides/image-update/#aws-elastic-container-registry Signed-off-by: lloydchang <lloydchang@gmail.com> Signed-off-by: Kingdon Barrett <kingdon@weave.works>

I noticed that some links might not be good, and all of the new links added in this PR were not following the convention of link targets in the footer. Correct those issues. Signed-off-by: Kingdon Barrett <kingdon@weave.works>

Signed-off-by: Kingdon Barrett <kingdon@weave.works>

I don't think API docs should refer to flux bootstrap

relu · 2022-01-07T11:28:25Z

docs/spec/v1beta1/imagerepositories.md

+
+Alternatively, the advice under [Other platforms][other platforms] below will also work for ECR.
+
+> You need to upgrade to Flux version 2 release [v0.19][Flux v0.19.0] that contains the image-reflector-controller release [v0.13.0][image-reflector-controller v0.13.0].


I suppose the right place for this documentation might actually be here. This section should be updated anyway since it's likely a lot of folks will prefer this method of authentication over the CronJob approach.

kingdonb · 2022-01-14T14:35:30Z

I am going to close this and reopen based on the current WIP in fluxcd/website#702

There are still maybe some valuable additions in here that have not been covered there already. For the most part, this PR is superseded by that one.

When fluxcd/website#702 merges, this patch will have been incorporated into the Image Update Guide. Add a link to the more directly relevant Image Update Guide here instead, since it has the example, and it doesn't belong in the API docs nor need to be repeated here, per the discussion from the earlier PR number fluxcd#193. Signed-off-by: Kingdon Barrett <kingdon@weave.works>

Squashed commits: follow the convention of using footer-style links I noticed that some links might not be good, and all of the new links added in this PR were not following the convention of link targets in the footer. Correct those issues. Signed-off-by: Kingdon Barrett <kingdon@weave.works> Update docs/spec/v1beta1/imagerepositories.md Signed-off-by: Kingdon Barrett <kingdon@weave.works> update docs Signed-off-by: Kingdon Barrett <kingdon@weave.works> Link to the Image Update guide When fluxcd/website#702 merges, this patch will have been incorporated into the Image Update Guide. Add a link to the more directly relevant Image Update Guide here instead, since it has the example, and it doesn't belong in the API docs nor need to be repeated here, per the discussion from the earlier PR number fluxcd#193. Signed-off-by: Kingdon Barrett <kingdon@weave.works> remove orphaned link URLs Signed-off-by: Kingdon Barrett <kingdon@weave.works>

Link to the Image Update guide When fluxcd/website#702 merges, this patch will have been incorporated into the Image Update Guide. Add a link to the more directly relevant Image Update Guide here instead, since it has the example, and it doesn't belong in the API docs nor need to be repeated here, per the discussion from the earlier PR number fluxcd#193. Signed-off-by: Kingdon Barrett <kingdon@weave.works> remove orphaned link URLs Signed-off-by: Kingdon Barrett <kingdon@weave.works> Signed-off-by: Kingdon Barrett <yebyen@gmail.com>

This was referenced Oct 31, 2021

Image Repositories fluxcd/website#601

Closed

Add note about AWS Autologin to Image Update guide fluxcd/website#611

Merged

lloydchang mentioned this pull request Oct 31, 2021

docs(image-reflector-controller/docs/spec/v1beta1/imagerepositories.md): elaborate on how to enable autologin for ECR kingdonb/image-reflector-controller#1

Merged

kingdonb force-pushed the explain-autologin-for-ecr branch from e34d8b7 to 0e2f925 Compare October 31, 2021 19:19

kingdonb commented Oct 31, 2021

View reviewed changes

docs/spec/v1beta1/imagerepositories.md Outdated Show resolved Hide resolved

lloydchang approved these changes Oct 31, 2021

View reviewed changes

relu previously approved these changes Nov 2, 2021

View reviewed changes

stefanprodan reviewed Nov 2, 2021

View reviewed changes

darkowlzz reviewed Nov 3, 2021

View reviewed changes

Kingdon Barrett and others added 5 commits December 16, 2021 13:52

Elaborate on how to enable autologin for ECR

5c30c4b

Signed-off-by: Kingdon Barrett <kingdon@weave.works>

follow the convention of using footer-style links

5dd39b7

I noticed that some links might not be good, and all of the new links added in this PR were not following the convention of link targets in the footer. Correct those issues. Signed-off-by: Kingdon Barrett <kingdon@weave.works>

Update docs/spec/v1beta1/imagerepositories.md

9f4d152

Signed-off-by: Kingdon Barrett <kingdon@weave.works>

update docs

4a7a3ce

Signed-off-by: Kingdon Barrett <kingdon@weave.works>

kingdonb force-pushed the explain-autologin-for-ecr branch from be77047 to 4a7a3ce Compare December 16, 2021 18:52

kingdonb mentioned this pull request Dec 29, 2021

Helm Operator 1.4.0 SIGSEGV fluxcd/helm-operator#628

Closed

stefanprodan requested a review from relu January 7, 2022 09:12

relu requested changes Jan 7, 2022

View reviewed changes

stefanprodan mentioned this pull request Jan 11, 2022

Get credentials from GCP/Azure when needed #194

Merged

kingdonb closed this Jan 14, 2022

kingdonb mentioned this pull request Jan 14, 2022

Clarify ECR Auto-login note, fix CanonicalImageName typos #219

Merged

kingdonb deleted the explain-autologin-for-ecr branch January 14, 2022 16:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Elaborate on how to enable autologin for ECR #193

Elaborate on how to enable autologin for ECR #193

kingdonb commented Oct 31, 2021

kingdonb commented Oct 31, 2021

relu left a comment

stefanprodan Nov 2, 2021

kingdonb Nov 2, 2021

relu Jan 7, 2022

squaremo commented Nov 2, 2021

squaremo commented Nov 2, 2021

stefanprodan commented Nov 2, 2021

kingdonb commented Nov 2, 2021

darkowlzz Nov 3, 2021 •

edited

Loading

kingdonb Dec 16, 2021

solms commented Nov 30, 2021

relu Jan 7, 2022

kingdonb commented Jan 14, 2022 •

edited

Loading


		Alternatively, the advice under [Other platforms][other platforms] below will also work for ECR.

		> You need to upgrade to Flux version 2 release [v0.19][Flux v0.19.0] that contains the image-reflector-controller release [v0.13.0][image-reflector-controller v0.13.0].

Elaborate on how to enable autologin for ECR #193

Elaborate on how to enable autologin for ECR #193

Conversation

kingdonb commented Oct 31, 2021

kingdonb commented Oct 31, 2021

relu left a comment

Choose a reason for hiding this comment

stefanprodan Nov 2, 2021

Choose a reason for hiding this comment

kingdonb Nov 2, 2021

Choose a reason for hiding this comment

relu Jan 7, 2022

Choose a reason for hiding this comment

squaremo commented Nov 2, 2021

squaremo commented Nov 2, 2021

stefanprodan commented Nov 2, 2021

kingdonb commented Nov 2, 2021

darkowlzz Nov 3, 2021 • edited Loading

Choose a reason for hiding this comment

kingdonb Dec 16, 2021

Choose a reason for hiding this comment

solms commented Nov 30, 2021

relu Jan 7, 2022

Choose a reason for hiding this comment

kingdonb commented Jan 14, 2022 • edited Loading

darkowlzz Nov 3, 2021 •

edited

Loading

kingdonb commented Jan 14, 2022 •

edited

Loading