Skip to content
This repository was archived by the owner on Nov 27, 2023. It is now read-only.

pass authprovider to buildkit session#1651

Merged
ndeloof merged 1 commit into
docker-archive:mainfrom
ndeloof:ghcr_credentials
May 7, 2021
Merged

pass authprovider to buildkit session#1651
ndeloof merged 1 commit into
docker-archive:mainfrom
ndeloof:ghcr_credentials

Conversation

@ndeloof

@ndeloof ndeloof commented May 6, 2021

Copy link
Copy Markdown
Collaborator

What I did
Aligned compose build code with https://github.com/docker/buildx/blob/master/commands/build.go

Related issue
#1545

@github-actions github-actions Bot added the local Local context (moby) label May 6, 2021
@ndeloof

ndeloof commented May 6, 2021

Copy link
Copy Markdown
Collaborator Author

@tonistiigi could you please confirm this is the correct way?
i.e. GetDriver already has an Auth parameter we pass docker config file, why do we also have to set this as Session option?

@ndeloof
ndeloof force-pushed the ghcr_credentials branch 3 times, most recently from c770dbf to 13b9341 Compare May 6, 2021 14:26

@gtardif gtardif left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM, Just pending @tonistiigi confirmation

@ndeloof

ndeloof commented May 6, 2021

Copy link
Copy Markdown
Collaborator Author

go.mod is killing me ...

@tonistiigi

Copy link
Copy Markdown

i.e. GetDriver already has an Auth parameter we pass docker config file, why do we also have to set this as Session option?

Yes, both are needed. GetDriver is only used when pulling buildkit image itself for creating the builder container one time. That image is public but we pass auth to avoid registry side rate-limiting.

@ndeloof

ndeloof commented May 7, 2021

Copy link
Copy Markdown
Collaborator Author

dependency issue:
buildx authprovider import introduced in this PR relies on containerd 1.5.x
kube backend relies on helm -> deislabs/oras -> containerd 1.4.x

1.4 -> 1.5 containerd updated PushContent signature to add some new parameters.
As Goland does not support polymorphism nor default parameter values (welcome into XXIth century) this is a backward compatibility break, which require a whole ecosystem of callers to update their code.

I can't find any possible workaround here 😱

@ndeloof
ndeloof force-pushed the ghcr_credentials branch 4 times, most recently from 52aa391 to 964a548 Compare May 7, 2021 09:06
Comment thread go.mod
replace github.com/jaguilar/vt100 => github.com/tonistiigi/vt100 v0.0.0-20190402012908-ad4c4a574305

// see https://github.com/helm/helm/pull/9675
replace helm.sh/helm/v3 => github.com/ndeloof/helm/v3 v3.1.0-rc.1.0.20210507084602-d0ceda767d95

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

A quick comment to explain this hack would be useful in the future...

@gtardif gtardif left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM with nit (comment on go mod replace)

…rameter

Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

local Local context (moby)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants