Handle case of missing GH variables (#492)

Closes #482 

I traced the problem to where buildx parses the cli args
([here](fa4461b9a1/util/buildflags/cache.go (L14))),
and confirmed it applies defaults based on GHA environment variables and
ignores the cacheTo/cacheFrom directive altogether when the variables
aren't available.

The fix is to ignore the GHA cache directive when it the upstream parser
ignores it.
This commit is contained in:
Eron Wright
2025-03-20 09:27:59 -07:00
committed by GitHub
parent f41c8c927d
commit 20f5f536dc
4 changed files with 100 additions and 12 deletions

View File

@@ -14,6 +14,7 @@
- Custom `# syntax=` directives no longer cause validation errors. (<https://github.com/pulumi/pulumi-docker-build/issues/300>) - Custom `# syntax=` directives no longer cause validation errors. (<https://github.com/pulumi/pulumi-docker-build/issues/300>)
- Upgrading docker-build no longer causes resource replacements. (<https://github.com/pulumi/pulumi-docker-build/issues/404>) - Upgrading docker-build no longer causes resource replacements. (<https://github.com/pulumi/pulumi-docker-build/issues/404>)
- Fixed leaking the GitHub actions secret in diff logs. (<https://github.com/pulumi/pulumi-docker-build/issues/403>) - Fixed leaking the GitHub actions secret in diff logs. (<https://github.com/pulumi/pulumi-docker-build/issues/403>)
- Provider panics when using Image resource with exec set to true. (<https://github.com/pulumi/pulumi-docker-build/issues/482>)
## 0.0.7 (2024-10-16) ## 0.0.7 (2024-10-16)

View File

@@ -468,9 +468,8 @@ func (c CacheFrom) validate(preview bool) (*controllerapi.CacheOptionsEntry, err
return nil, err return nil, err
} }
if len(parsed) == 0 { if len(parsed) == 0 {
// This can happen for example if we have a GHA cache but no GitHub // This can happen for example if we have a GHA cache configuration but no GitHub
// environment variables set. // environment variables set. Ignore the cacheFrom entry in this case.
// Shouldn't happen...
return nil, nil return nil, nil
} }
return parsed[0], nil return parsed[0], nil
@@ -677,9 +676,8 @@ func (c CacheTo) validate(preview bool) (*controllerapi.CacheOptionsEntry, error
return nil, err return nil, err
} }
if len(parsed) == 0 { if len(parsed) == 0 {
// This can happen for example if we have a GHA cache but no GitHub // This can happen for example if we have a GHA cache configuration but no GitHub
// environment variables set. // environment variables set. Ignore the cacheTo entry in this case.
// Shouldn't happen...
return nil, nil return nil, nil
} }
return parsed[0], nil return parsed[0], nil

View File

@@ -576,8 +576,10 @@ func (ia *ImageArgs) validate(supportsMultipleExports, preview bool) (controller
multierr = errors.Join(multierr, newCheckFailure(err, "exports[%d]", idx)) multierr = errors.Join(multierr, newCheckFailure(err, "exports[%d]", idx))
continue continue
} }
if exp != nil {
exports = append(exports, exp) exports = append(exports, exp)
} }
}
platforms := []string{} platforms := []string{}
for idx, p := range normalized.Platforms { for idx, p := range normalized.Platforms {
@@ -586,8 +588,10 @@ func (ia *ImageArgs) validate(supportsMultipleExports, preview bool) (controller
multierr = errors.Join(multierr, newCheckFailure(err, "platforms[%d]", idx)) multierr = errors.Join(multierr, newCheckFailure(err, "platforms[%d]", idx))
continue continue
} }
if platform != "" {
platforms = append(platforms, platform) platforms = append(platforms, platform)
} }
}
cacheFrom := []*controllerapi.CacheOptionsEntry{} cacheFrom := []*controllerapi.CacheOptionsEntry{}
for idx, c := range normalized.CacheFrom { for idx, c := range normalized.CacheFrom {
@@ -599,8 +603,10 @@ func (ia *ImageArgs) validate(supportsMultipleExports, preview bool) (controller
multierr = errors.Join(multierr, newCheckFailure(err, "cacheFrom[%d]", idx)) multierr = errors.Join(multierr, newCheckFailure(err, "cacheFrom[%d]", idx))
continue continue
} }
if cache != nil {
cacheFrom = append(cacheFrom, cache) cacheFrom = append(cacheFrom, cache)
} }
}
cacheTo := []*controllerapi.CacheOptionsEntry{} cacheTo := []*controllerapi.CacheOptionsEntry{}
for idx, c := range normalized.CacheTo { for idx, c := range normalized.CacheTo {
@@ -612,8 +618,10 @@ func (ia *ImageArgs) validate(supportsMultipleExports, preview bool) (controller
multierr = errors.Join(multierr, newCheckFailure(err, "cacheTo[%d]", idx)) multierr = errors.Join(multierr, newCheckFailure(err, "cacheTo[%d]", idx))
continue continue
} }
if cache != nil {
cacheTo = append(cacheTo, cache) cacheTo = append(cacheTo, cache)
} }
}
ssh := []*controllerapi.SSH{} ssh := []*controllerapi.SSH{}
for idx, s := range normalized.SSH { for idx, s := range normalized.SSH {
@@ -622,8 +630,10 @@ func (ia *ImageArgs) validate(supportsMultipleExports, preview bool) (controller
multierr = errors.Join(multierr, newCheckFailure(err, "ssh[%d]", idx)) multierr = errors.Join(multierr, newCheckFailure(err, "ssh[%d]", idx))
continue continue
} }
if ss != nil {
ssh = append(ssh, ss) ssh = append(ssh, ss)
} }
}
for idx, t := range normalized.Tags { for idx, t := range normalized.Tags {
if _, err := reference.Parse(t); err != nil { if _, err := reference.Parse(t); err != nil {

View File

@@ -25,6 +25,7 @@ import (
_ "github.com/docker/buildx/driver/docker-container" _ "github.com/docker/buildx/driver/docker-container"
"github.com/distribution/reference" "github.com/distribution/reference"
pb "github.com/docker/buildx/controller/pb"
"github.com/moby/buildkit/client" "github.com/moby/buildkit/client"
"github.com/moby/buildkit/exporter/containerimage/exptypes" "github.com/moby/buildkit/exporter/containerimage/exptypes"
"github.com/regclient/regclient/types/descriptor" "github.com/regclient/regclient/types/descriptor"
@@ -818,7 +819,6 @@ func TestImageDiff(t *testing.T) {
} }
func TestValidateImageArgs(t *testing.T) { func TestValidateImageArgs(t *testing.T) {
t.Parallel()
t.Run("invalid inputs", func(t *testing.T) { t.Run("invalid inputs", func(t *testing.T) {
t.Parallel() t.Parallel()
args := ImageArgs{ args := ImageArgs{
@@ -917,6 +917,85 @@ func TestValidateImageArgs(t *testing.T) {
assert.Len(t, opts.Exports, 0) assert.Len(t, opts.Exports, 0)
}) })
t.Run("environment variables", func(t *testing.T) {
tests := []struct {
name string
envs map[string]string
args ImageArgs
wantCacheFrom *pb.CacheOptionsEntry
wantCacheTo *pb.CacheOptionsEntry
}{
{
name: "gha environment",
envs: map[string]string{
"ACTIONS_CACHE_URL": "test-cache-url",
"ACTIONS_RUNTIME_TOKEN": "test-runtime-token",
},
args: ImageArgs{
Context: &BuildContext{Context: Context{Location: "testdata/noop"}},
CacheFrom: []CacheFrom{{GHA: &CacheFromGitHubActions{}}},
CacheTo: []CacheTo{{GHA: &CacheToGitHubActions{
CacheFromGitHubActions: CacheFromGitHubActions{},
}}},
},
wantCacheFrom: &pb.CacheOptionsEntry{
Type: "gha",
Attrs: map[string]string{
"token": "test-runtime-token",
"url": "test-cache-url",
},
},
wantCacheTo: &pb.CacheOptionsEntry{
Type: "gha",
Attrs: map[string]string{
"token": "test-runtime-token",
"url": "test-cache-url",
},
},
},
{
name: "non-gha environment",
envs: map[string]string{
"ACTIONS_CACHE_URL": "",
"ACTIONS_RUNTIME_TOKEN": "",
},
args: ImageArgs{
Context: &BuildContext{Context: Context{Location: "testdata/noop"}},
CacheFrom: []CacheFrom{{GHA: &CacheFromGitHubActions{}}},
CacheTo: []CacheTo{{GHA: &CacheToGitHubActions{
CacheFromGitHubActions: CacheFromGitHubActions{},
}}},
},
wantCacheFrom: nil,
wantCacheTo: nil,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
for k, v := range tt.envs {
t.Setenv(k, v)
}
validate := func(preview bool) {
opts, err := tt.args.validate(true, preview)
require.NoError(t, err)
if tt.wantCacheFrom != nil {
assert.Equal(t, tt.wantCacheFrom, opts.CacheFrom[0])
} else {
assert.Len(t, opts.CacheFrom, 0)
}
if tt.wantCacheTo != nil {
assert.Equal(t, tt.wantCacheTo, opts.CacheTo[0])
} else {
assert.Len(t, opts.CacheTo, 0)
}
}
validate(true)
validate(false)
})
}
})
t.Run("multiple exports pre-0.13", func(t *testing.T) { t.Run("multiple exports pre-0.13", func(t *testing.T) {
t.Parallel() t.Parallel()
args := ImageArgs{ args := ImageArgs{