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

Add support for Docker default config file #900

Merged
merged 2 commits into from
Oct 12, 2022
Merged

Conversation

samj1912
Copy link
Contributor

@samj1912 samj1912 commented Oct 2, 2022

Fix #871
Fix #872
Fix #606

This PR guts the custom docker image/digest logic and replaces it with GGCR. This allows us to have a robust docker validation logic.

Test

To test this pull request, you can run the following commands:

sources:
  tag:
    kind: dockerimage
    spec:
      image: ghcr.io/updatecli/updatecli
  digest:
    kind: dockerdigest
    spec:
      image: ghcr.io/updatecli/updatecli
      tag: v0.34.0
./dist/updatecli_darwin_arm64/updatecli diff --config config.yaml


+++++++++++
+ PREPARE +
+++++++++++

Loading Pipeline "config.yaml"

SCM repository retrieved: 0


++++++++++++++++++
+ AUTO DISCOVERY +
++++++++++++++++++



++++++++++++
+ PIPELINE +
++++++++++++



###############
# CONFIG.YAML #
###############


SOURCES
=======

digest
------
✔ Docker Image Tag ghcr.io/updatecli/updatecli:v0.34.0 resolved to digest ghcr.io/updatecli/updatecli@sha256:779abc2c7a46c0cc0d568bac6e6d8b81c9b3255bb025a5f421d0911a82e3d4c5

tag
---
Searching for version matching pattern "latest"
✔ Docker Image Tag "v0.34.0" found matching pattern "latest"

=============================

REPORTS:


- CONFIG.YAML:
	Source:
		✔ [digest]  (kind: dockerdigest)
		✔ [tag]  (kind: dockerimage)

Run Summary
===========
Pipeline(s) run:
  * Changed:	0
  * Failed:	0
  * Skipped:	1
  * Succeeded:	0
  * Total:	1

Additional Information

Tradeoff

Potential improvement

@olblak
Copy link
Member

olblak commented Oct 11, 2022

Nice work, I didn't realize how far you went. Indeed I need some e2e tests for different registries like ghcr.io dockerhub and quay

@olblak olblak marked this pull request as ready for review October 11, 2022 18:02
@olblak olblak marked this pull request as draft October 11, 2022 18:02
@olblak
Copy link
Member

olblak commented Oct 11, 2022

I did some initial testing and I didn't find any regression, I need to find some ways to test with other registries than dockerhub or ghcr.io

@olblak olblak marked this pull request as ready for review October 11, 2022 18:17
@olblak
Copy link
Member

olblak commented Oct 11, 2022

I confirm that your code fallback to the docker configuration file to retrieve credentials.
I don't have permission to push on your local branch, so I'll probably move forward with your pullrequest and the e2e tests that I have locally

@samj1912
Copy link
Contributor Author

The only 'regression' might be the registry for how dockerhub is resolved. It will be resolved to index.docker.io

@olblak
Copy link
Member

olblak commented Oct 12, 2022

The only 'regression' might be the registry for how dockerhub is resolved. It will be resolved to index.docker.io

Which I don't think is a problem at all. If I recall correctly that URL was needed for the previous code which is now replaced by GGCR

olblak
olblak previously approved these changes Oct 12, 2022
@olblak
Copy link
Member

olblak commented Oct 12, 2022

I am renaming the pullrequest to "Add support for Docker default config file" to highlight that this pullrequest is not only a great refactoring by fixing various issues but it's great enhancement that will benefit to autodiscovery feature.

@olblak olblak changed the title Improve docker image/digest support Add support for Docker default config file Oct 12, 2022
@olblak olblak added enhancement New feature or request resource-dockerdigest Resource of kind dockerDigest resource-docker Resource of kind Docker Image resource-dockerfile Resource of kind Dockerfile labels Oct 12, 2022
@olblak olblak added this to the 0.36.0 milestone Oct 12, 2022
@samj1912
Copy link
Contributor Author

@olblak fixed the failing registry test and rebased. Should be all good to go now.

@olblak olblak merged commit d15e672 into updatecli:main Oct 12, 2022
@samj1912 samj1912 deleted the docker branch October 12, 2022 19:16
@olblak olblak mentioned this pull request Jun 7, 2023
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request resource-docker Resource of kind Docker Image resource-dockerdigest Resource of kind dockerDigest resource-dockerfile Resource of kind Dockerfile
Projects
None yet
2 participants