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.

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

fix: avoid short names in Dockerfiles #5474

Merged
merged 2 commits into from
Mar 24, 2021
Merged

Conversation

sbose78
Copy link
Contributor

@sbose78 sbose78 commented Mar 22, 2021

  • Using short names is subject to the risk of hitting squatted registry namespaces
  • If a user's configuration is set to pull images from somewhere other then Dockerhub, the user may unknowingly pull a malicious image.

possibly part of argoproj/argoproj#40

Signed-off-by: Shoubhik Bose shbose@redhat.com

Checklist:

Signed-off-by: Shoubhik Bose <shbose@redhat.com>
@sbose78 sbose78 changed the title fix: avoid short names in Dockerfile fix: avoid short names in Dockerfiles Mar 22, 2021
@codecov
Copy link

codecov bot commented Mar 22, 2021

Codecov Report

Merging #5474 (a08146e) into master (5bd7ce8) will decrease coverage by 0.01%.
The diff coverage is n/a.

❗ Current head a08146e differs from pull request most recent head d257038. Consider uploading reports for the commit d257038 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5474      +/-   ##
==========================================
- Coverage   16.49%   16.47%   -0.02%     
==========================================
  Files         243      243              
  Lines       43774    43774              
==========================================
- Hits         7219     7212       -7     
- Misses      35573    35578       +5     
- Partials      982      984       +2     
Impacted Files Coverage Δ
workflow/metrics/server.go 12.76% <0.00%> (-4.26%) ⬇️
cmd/argoexec/commands/emissary.go 48.43% <0.00%> (-1.57%) ⬇️
workflow/controller/operator.go 70.24% <0.00%> (-0.28%) ⬇️
cmd/argo/commands/get.go 56.66% <0.00%> (+0.66%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5bd7ce8...d257038. Read the comment docs.

Signed-off-by: Shoubhik Bose <shbose@redhat.com>
@alexec
Copy link
Contributor

alexec commented Mar 22, 2021

I think you there is a kustomize edit set image somewhere that will also need updating?

@sbose78
Copy link
Contributor Author

sbose78 commented Mar 22, 2021

I think you there is a kustomize edit set image somewhere that will also need updating?

Sorry, not sure what you are referring to :(

Copy link
Member

@wanghong230 wanghong230 left a comment

Choose a reason for hiding this comment

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

LGTM. There is no harm.

@alexec
Copy link
Contributor

alexec commented Mar 24, 2021

Ah. I think we changed from edit set image to this:

https://github.com/argoproj/argo-workflows/blob/master/hack/update-image-tags.sh#L7

So no change needed.

@alexec alexec merged commit c13755b into argoproj:master Mar 24, 2021
This was referenced Mar 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants