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

Custom builder should be able to return image digest #4624

Open
maxmiles39 opened this issue Aug 4, 2020 · 9 comments
Open

Custom builder should be able to return image digest #4624

maxmiles39 opened this issue Aug 4, 2020 · 9 comments
Labels
area/deploy build/custom kind/bug Something isn't working priority/p3 agreed that this would be good to have, but no one is available at the moment.

Comments

@maxmiles39
Copy link

maxmiles39 commented Aug 4, 2020

For customized build, user is responsible for build and publish the image. Skaffold do not need to do the extra validation, or user should be able to disable it. Otherwise, user have to make the extra configuration to allow skaffold to access the image, for example, we need to make the registry public accessible.

Expected behavior

Skaffold skip the validation for customized build

Actual behavior

Skaffold is validating the image and extra config is required.
.

Information

  • Contents of skaffold.yaml:
apiVersion: skaffold/v2beta4
kind: Config
build:
  tagPolicy:
    sha256: {}
  artifacts:
  - image: internal-registry/skaffold-custom
    custom:
      buildCommand: ./build.sh
@MarlonGamez
Copy link
Contributor

Hi @dwctua, when you get the chance, would you be able to share the error message you're seeing when trying to build and push without allowing skaffold access to the image registry? Thanks :)

@maxmiles39
Copy link
Author

see error below after run skaffold dev

exiting dev mode because first build failed: couldn't build "internal-registry/skaffold-custom": getting image: GET https://index.docker.io/v2
/internal-registry/skaffold-custom/manifests/latest: UNAUTHORIZED: authentication required; [map[Action:pull Class: Name:internal-registry/ska
ffold-custom Type:repository]]

@maxmiles39
Copy link
Author

maxmiles39 commented Aug 11, 2020

I updated the skaffold.yaml. the build.sh can be an empty file for the demo. the idea is let build.sh handle image build and push, but use skaffold as file watcher to trigger the build and re-deployment.

@MarlonGamez MarlonGamez added kind/bug Something isn't working area/cache labels Aug 13, 2020
@MarlonGamez
Copy link
Contributor

It looks like the error is coming from a caching related step of skaffold where we add a tag to the remote image. If you try to run skaffold with --cache-artifacts=false, does your issue still occur? I agree that skaffold shouldn't have to have access to the registry

@maxmiles39
Copy link
Author

I'm still getting the same error by running command: skaffold run --cache-artifacts=false

@MarlonGamez
Copy link
Contributor

Ah okay, thanks for trying it out. I'll bring this issue up in our team discussion soon 👍🏽

@MarlonGamez MarlonGamez added the triage/discuss Items for discussion label Aug 14, 2020
@nkubala nkubala added area/deploy build/custom priority/p2 May take a couple of releases and removed triage/discuss Items for discussion labels Aug 24, 2020
@tejal29
Copy link
Member

tejal29 commented Aug 24, 2020

@dwctua, skaffold needs to fetch the remote digest associated with the image to substitute all image names in the manifests.

One thing, we could do is, expose another environment variables where e.g. IMAGE_REF which custom builders can set for skaffold to fetch image remote id.

@maxmiles39
Copy link
Author

I like the environment variable, this will remove the dependency for docker registry. As side note, Skaffold is using both tag and digest, it is also causing issue in openshift.

@briandealwis briandealwis changed the title remove skaffold image validation for custom build Custom builder should be able to return image digest Sep 1, 2020
@nkubala nkubala added this to the Icebox milestone Sep 1, 2020
@MarlonGamez
Copy link
Contributor

bumping the priority on this as I don't think our team has the bandwidth to push this through right now. PRs welcome!

@MarlonGamez MarlonGamez added priority/p3 agreed that this would be good to have, but no one is available at the moment. and removed priority/p2 May take a couple of releases labels Nov 9, 2020
@nkubala nkubala removed this from the Icebox [P2+] milestone May 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/deploy build/custom kind/bug Something isn't working priority/p3 agreed that this would be good to have, but no one is available at the moment.
Projects
None yet
Development

No branches or pull requests

4 participants