From e8636078287416100a8da9e42a95591f4851fb00 Mon Sep 17 00:00:00 2001 From: Brian Goff Date: Fri, 13 Dec 2024 16:37:45 -0800 Subject: [PATCH 1/3] windows: Remove duplicate target parsing Signed-off-by: Brian Goff --- frontend/windows/handle_container.go | 35 ++++++---------------------- 1 file changed, 7 insertions(+), 28 deletions(-) diff --git a/frontend/windows/handle_container.go b/frontend/windows/handle_container.go index 8d58207f2..bd88a2310 100644 --- a/frontend/windows/handle_container.go +++ b/frontend/windows/handle_container.go @@ -11,7 +11,6 @@ import ( "github.com/Azure/dalec/frontend" "github.com/moby/buildkit/client/llb" "github.com/moby/buildkit/client/llb/sourceresolver" - "github.com/moby/buildkit/frontend/dockerui" gwclient "github.com/moby/buildkit/frontend/gateway/client" ocispecs "github.com/opencontainers/image-spec/specs-go/v1" "github.com/pkg/errors" @@ -44,16 +43,6 @@ func handleContainer(ctx context.Context, client gwclient.Client) (*gwclient.Res return nil, nil, fmt.Errorf("error validating windows spec: %w", err) } - bc, err := dockerui.NewClient(client) - if err != nil { - return nil, nil, err - } - - targetPlatform, err := getTargetPlatform(bc) - if err != nil { - return nil, nil, err - } - pg := dalec.ProgressGroup("Build windows container: " + spec.Name) worker, err := distroConfig.Worker(sOpt, pg) if err != nil { @@ -66,7 +55,12 @@ func handleContainer(ctx context.Context, client gwclient.Client) (*gwclient.Res } baseImgName := getBaseOutputImage(spec, targetKey, defaultBaseImage) - baseImage := llb.Image(baseImgName, llb.Platform(targetPlatform)) + + if platform == nil { + platform = &defaultPlatform + } + + baseImage := llb.Image(baseImgName, llb.Platform(*platform)) out := baseImage. File(llb.Copy(bin, "/", windowsSystemDir)). @@ -90,7 +84,7 @@ func handleContainer(ctx context.Context, client gwclient.Client) (*gwclient.Res } _, _, dt, err := client.ResolveImageConfig(ctx, imgRef, sourceresolver.Opt{ - Platform: &targetPlatform, + Platform: platform, }) if err != nil { return nil, nil, errors.Wrap(err, "could not resolve base image config") @@ -133,21 +127,6 @@ func copySymlinks(post *dalec.PostInstall) llb.StateOption { } -func getTargetPlatform(bc *dockerui.Client) (ocispecs.Platform, error) { - platform := defaultPlatform - - switch len(bc.TargetPlatforms) { - case 0: - case 1: - platform = bc.TargetPlatforms[0] - default: - return ocispecs.Platform{}, - fmt.Errorf("multiple target supplied for build: %v. note: only amd64 is supported for windows outputs", bc.TargetPlatforms) - } - - return platform, nil -} - func getBaseOutputImage(spec *dalec.Spec, target, defaultBase string) string { baseRef := defaultBase if spec.Targets[target].Image != nil && spec.Targets[target].Image.Base != "" { From d1d010e5fc50da3df2b9127c19f450c9f2458bde Mon Sep 17 00:00:00 2001 From: Brian Goff Date: Wed, 18 Dec 2024 15:35:21 -0800 Subject: [PATCH 2/3] Allow spec loaders to inject allowed args This makes it so frontends can have their own args that the dalec core does not need to know about. This also moves the custom args for windowscross local to that implementation. Signed-off-by: Brian Goff --- frontend/build.go | 36 +++++++++++++- frontend/mux.go | 9 +++- load.go | 122 ++++++++++++++++++++++++++-------------------- load_test.go | 10 +++- 4 files changed, 121 insertions(+), 56 deletions(-) diff --git a/frontend/build.go b/frontend/build.go index 146133b83..86a2e1f83 100644 --- a/frontend/build.go +++ b/frontend/build.go @@ -15,7 +15,39 @@ import ( "github.com/pkg/errors" ) -func LoadSpec(ctx context.Context, client *dockerui.Client, platform *ocispecs.Platform) (*dalec.Spec, error) { +type LoadConfig struct { + SubstituteOpts []dalec.SubstituteOpt +} + +type LoadOpt func(*LoadConfig) + +func WithAllowArgs(args ...string) LoadOpt { + return func(cfg *LoadConfig) { + set := make(map[string]struct{}, len(args)) + for _, arg := range args { + set[arg] = struct{}{} + } + cfg.SubstituteOpts = append(cfg.SubstituteOpts, func(cfg *dalec.SubstituteConfig) { + orig := cfg.AllowArg + + cfg.AllowArg = func(key string) bool { + if orig != nil && orig(key) { + return true + } + _, ok := set[key] + return ok + } + }) + } +} + +func LoadSpec(ctx context.Context, client *dockerui.Client, platform *ocispecs.Platform, opts ...LoadOpt) (*dalec.Spec, error) { + cfg := LoadConfig{} + + for _, o := range opts { + o(&cfg) + } + src, err := client.ReadEntrypoint(ctx, "Dockerfile") if err != nil { return nil, fmt.Errorf("could not read spec file: %w", err) @@ -35,7 +67,7 @@ func LoadSpec(ctx context.Context, client *dockerui.Client, platform *ocispecs.P fillPlatformArgs("TARGET", args, *platform) fillPlatformArgs("BUILD", args, client.BuildPlatforms[0]) - if err := spec.SubstituteArgs(args); err != nil { + if err := spec.SubstituteArgs(args, cfg.SubstituteOpts...); err != nil { return nil, errors.Wrap(err, "error resolving build args") } return spec, nil diff --git a/frontend/mux.go b/frontend/mux.go index 8428a8f6d..384fffa3e 100644 --- a/frontend/mux.go +++ b/frontend/mux.go @@ -204,7 +204,14 @@ func (m *BuildMux) loadSpec(ctx context.Context, client gwclient.Client) (*dalec } // Note: this is not suitable for passing to builds since it does not have platform information - spec, err := LoadSpec(ctx, dc, nil) + spec, err := LoadSpec(ctx, dc, nil, func(cfg *LoadConfig) { + cfg.SubstituteOpts = append(cfg.SubstituteOpts, func(cfg *dalec.SubstituteConfig) { + // Allow any args here since we aren't trying to validate the spec at this point. + cfg.AllowArg = func(string) bool { + return true + } + }) + }) if err != nil { return nil, err } diff --git a/load.go b/load.go index 79236ee2d..4eaa717f3 100644 --- a/load.go +++ b/load.go @@ -58,7 +58,7 @@ func (m envGetterMap) Keys() []string { return maps.Keys(m) } -func expandArgs(lex *shell.Lex, s string, args map[string]string) (string, error) { +func expandArgs(lex *shell.Lex, s string, args map[string]string, allowArg func(key string) bool) (string, error) { result, err := lex.ProcessWordWithMatches(s, envGetterMap(args)) if err != nil { return "", err @@ -66,7 +66,7 @@ func expandArgs(lex *shell.Lex, s string, args map[string]string) (string, error var errs []error for m := range result.Unmatched { - if !knownArg(m) { + if !knownArg(m) && !allowArg(m) { errs = append(errs, fmt.Errorf(`build arg "%s" not declared`, m)) continue } @@ -79,7 +79,7 @@ func expandArgs(lex *shell.Lex, s string, args map[string]string) (string, error return result.Result, goerrors.Join(errs...) } -func (s *Source) substituteBuildArgs(args map[string]string) error { +func (s *Source) substituteBuildArgs(args map[string]string, allowArg func(key string) bool) error { lex := shell.NewLex('\\') // force the shell lexer to skip unresolved env vars so they aren't // replaced with "" @@ -92,7 +92,7 @@ func (s *Source) substituteBuildArgs(args map[string]string) error { switch { case s.DockerImage != nil: - updated, err := expandArgs(lex, s.DockerImage.Ref, args) + updated, err := expandArgs(lex, s.DockerImage.Ref, args, allowArg) if err != nil { appendErr(fmt.Errorf("error performing shell expansion on docker image ref: %w", err)) } @@ -100,50 +100,50 @@ func (s *Source) substituteBuildArgs(args map[string]string) error { if s.DockerImage.Cmd != nil { for _, mnt := range s.DockerImage.Cmd.Mounts { - err := mnt.Spec.substituteBuildArgs(args) + err := mnt.Spec.substituteBuildArgs(args, allowArg) if err != nil { appendErr(fmt.Errorf("error performing shell expansion on docker image mount: %w", err)) } } } case s.Git != nil: - updated, err := expandArgs(lex, s.Git.URL, args) + updated, err := expandArgs(lex, s.Git.URL, args, allowArg) s.Git.URL = updated if err != nil { appendErr(err) } - updated, err = expandArgs(lex, s.Git.Commit, args) + updated, err = expandArgs(lex, s.Git.Commit, args, allowArg) s.Git.Commit = updated if err != nil { appendErr(err) } case s.HTTP != nil: - updated, err := expandArgs(lex, s.HTTP.URL, args) + updated, err := expandArgs(lex, s.HTTP.URL, args, allowArg) if err != nil { appendErr(err) } s.HTTP.URL = updated case s.Context != nil: - updated, err := expandArgs(lex, s.Context.Name, args) + updated, err := expandArgs(lex, s.Context.Name, args, allowArg) s.Context.Name = updated if err != nil { appendErr(err) } case s.Build != nil: - err := s.Build.Source.substituteBuildArgs(args) + err := s.Build.Source.substituteBuildArgs(args, allowArg) if err != nil { appendErr(err) } - updated, err := expandArgs(lex, s.Build.DockerfilePath, args) + updated, err := expandArgs(lex, s.Build.DockerfilePath, args, allowArg) if err != nil { appendErr(err) } s.Build.DockerfilePath = updated - updated, err = expandArgs(lex, s.Build.Target, args) + updated, err = expandArgs(lex, s.Build.Target, args, allowArg) if err != nil { appendErr(err) } @@ -255,7 +255,25 @@ func (s *Source) validate(failContext ...string) (retErr error) { var errUnknownArg = errors.New("unknown arg") -func (s *Spec) SubstituteArgs(env map[string]string) error { +type SubstituteConfig struct { + AllowArg func(string) bool +} + +type SubstituteOpt func(*SubstituteConfig) + +func (s *Spec) SubstituteArgs(env map[string]string, opts ...SubstituteOpt) error { + var cfg SubstituteConfig + + for _, o := range opts { + o(&cfg) + } + + if cfg.AllowArg == nil { + cfg.AllowArg = func(string) bool { + return false + } + } + lex := shell.NewLex('\\') // force the shell lexer to skip unresolved env vars so they aren't // replaced with "" @@ -272,7 +290,7 @@ func (s *Spec) SubstituteArgs(env map[string]string) error { } for k, v := range env { if _, ok := args[k]; !ok { - if !knownArg(k) { + if !knownArg(k) && !cfg.AllowArg(k) { appendErr(fmt.Errorf("%w: %q", errUnknownArg, k)) } @@ -285,31 +303,31 @@ func (s *Spec) SubstituteArgs(env map[string]string) error { } for name, src := range s.Sources { - if err := src.substituteBuildArgs(args); err != nil { + if err := src.substituteBuildArgs(args, cfg.AllowArg); err != nil { appendErr(fmt.Errorf("error performing shell expansion on source %q: %w", name, err)) } if src.DockerImage != nil { - if err := src.DockerImage.Cmd.processBuildArgs(lex, args, name); err != nil { + if err := src.DockerImage.Cmd.processBuildArgs(lex, args, name, cfg.AllowArg); err != nil { appendErr(fmt.Errorf("error performing shell expansion on source %q: %w", name, err)) } } s.Sources[name] = src } - updated, err := expandArgs(lex, s.Version, args) + updated, err := expandArgs(lex, s.Version, args, cfg.AllowArg) if err != nil { appendErr(fmt.Errorf("error performing shell expansion on version: %w", err)) } s.Version = updated - updated, err = expandArgs(lex, s.Revision, args) + updated, err = expandArgs(lex, s.Revision, args, cfg.AllowArg) if err != nil { appendErr(fmt.Errorf("error performing shell expansion on revision: %w", err)) } s.Revision = updated for k, v := range s.Build.Env { - updated, err := expandArgs(lex, v, args) + updated, err := expandArgs(lex, v, args, cfg.AllowArg) if err != nil { appendErr(fmt.Errorf("error performing shell expansion on env var %q: %w", k, err)) } @@ -317,7 +335,7 @@ func (s *Spec) SubstituteArgs(env map[string]string) error { } if s.Build.NetworkMode != "" { - updated, err := expandArgs(lex, s.Build.NetworkMode, args) + updated, err := expandArgs(lex, s.Build.NetworkMode, args, cfg.AllowArg) if err != nil { appendErr(fmt.Errorf("error performing shell expansion on build network mode: %s: %w", s.Build.NetworkMode, err)) } @@ -326,26 +344,26 @@ func (s *Spec) SubstituteArgs(env map[string]string) error { for i, step := range s.Build.Steps { bs := &step - if err := bs.processBuildArgs(lex, args, i); err != nil { + if err := bs.processBuildArgs(lex, args, i, cfg.AllowArg); err != nil { appendErr(fmt.Errorf("error performing shell expansion on build step %d: %w", i, err)) } s.Build.Steps[i] = *bs } for _, t := range s.Tests { - if err := t.processBuildArgs(lex, args, t.Name); err != nil { + if err := t.processBuildArgs(lex, args, t.Name, cfg.AllowArg); err != nil { appendErr(fmt.Errorf("error performing shell expansion on test %q: %w", t.Name, err)) } } for name, t := range s.Targets { - if err := t.processBuildArgs(name, lex, args); err != nil { + if err := t.processBuildArgs(name, lex, args, cfg.AllowArg); err != nil { appendErr(fmt.Errorf("error processing build args for target %q: %w", name, err)) } } if s.PackageConfig != nil { - if err := s.PackageConfig.processBuildArgs(lex, args); err != nil { + if err := s.PackageConfig.processBuildArgs(lex, args, cfg.AllowArg); err != nil { appendErr(fmt.Errorf("could not process build args for base spec package config: %w", err)) } } @@ -389,10 +407,10 @@ func stripXFields(dt []byte) ([]byte, error) { return yaml.Marshal(obj) } -func (s *BuildStep) processBuildArgs(lex *shell.Lex, args map[string]string, i int) error { +func (s *BuildStep) processBuildArgs(lex *shell.Lex, args map[string]string, i int, allowArg func(string) bool) error { var errs []error for k, v := range s.Env { - updated, err := expandArgs(lex, v, args) + updated, err := expandArgs(lex, v, args, allowArg) if err != nil { errs = append(errs, fmt.Errorf("error performing shell expansion on env var %q for step %d: %w", k, i, err)) } @@ -401,7 +419,7 @@ func (s *BuildStep) processBuildArgs(lex *shell.Lex, args map[string]string, i i return goerrors.Join(errs...) } -func (c *Command) processBuildArgs(lex *shell.Lex, args map[string]string, name string) error { +func (c *Command) processBuildArgs(lex *shell.Lex, args map[string]string, name string, allowArg func(string) bool) error { if c == nil { return nil } @@ -412,12 +430,12 @@ func (c *Command) processBuildArgs(lex *shell.Lex, args map[string]string, name } for _, s := range c.Mounts { - if err := s.Spec.substituteBuildArgs(args); err != nil { + if err := s.Spec.substituteBuildArgs(args, allowArg); err != nil { appendErr(fmt.Errorf("error performing shell expansion on source ref %q: %w", name, err)) } } for k, v := range c.Env { - updated, err := expandArgs(lex, v, args) + updated, err := expandArgs(lex, v, args, allowArg) if err != nil { appendErr(fmt.Errorf("error performing shell expansion on env var %q for source %q: %w", k, name, err)) } @@ -425,7 +443,7 @@ func (c *Command) processBuildArgs(lex *shell.Lex, args map[string]string, name } for i, step := range c.Steps { for k, v := range step.Env { - updated, err := expandArgs(lex, v, args) + updated, err := expandArgs(lex, v, args, allowArg) if err != nil { appendErr(fmt.Errorf("error performing shell expansion on env var %q for source %q: %w", k, name, err)) } @@ -560,34 +578,34 @@ func validatePatch(patch PatchSpec, patchSrc Source) error { return nil } -func (c *CheckOutput) processBuildArgs(lex *shell.Lex, args map[string]string) error { +func (c *CheckOutput) processBuildArgs(lex *shell.Lex, args map[string]string, allowArg func(string) bool) error { for i, contains := range c.Contains { - updated, err := expandArgs(lex, contains, args) + updated, err := expandArgs(lex, contains, args, allowArg) if err != nil { return errors.Wrap(err, "error performing shell expansion on contains") } c.Contains[i] = updated } - updated, err := expandArgs(lex, c.EndsWith, args) + updated, err := expandArgs(lex, c.EndsWith, args, allowArg) if err != nil { return errors.Wrap(err, "error performing shell expansion on endsWith") } c.EndsWith = updated - updated, err = expandArgs(lex, c.Matches, args) + updated, err = expandArgs(lex, c.Matches, args, allowArg) if err != nil { return errors.Wrap(err, "error performing shell expansion on matches") } c.Matches = updated - updated, err = expandArgs(lex, c.Equals, args) + updated, err = expandArgs(lex, c.Equals, args, allowArg) if err != nil { return errors.Wrap(err, "error performing shell expansion on equals") } c.Equals = updated - updated, err = expandArgs(lex, c.StartsWith, args) + updated, err = expandArgs(lex, c.StartsWith, args, allowArg) if err != nil { return errors.Wrap(err, "error performing shell expansion on startsWith") } @@ -595,21 +613,21 @@ func (c *CheckOutput) processBuildArgs(lex *shell.Lex, args map[string]string) e return nil } -func (c *TestSpec) processBuildArgs(lex *shell.Lex, args map[string]string, name string) error { +func (c *TestSpec) processBuildArgs(lex *shell.Lex, args map[string]string, name string, allowArg func(string) bool) error { var errs []error appendErr := func(err error) { errs = append(errs, err) } for _, s := range c.Mounts { - err := s.Spec.substituteBuildArgs(args) + err := s.Spec.substituteBuildArgs(args, allowArg) if err != nil { appendErr(fmt.Errorf("error performing shell expansion on source ref %q: %w", name, err)) } } for k, v := range c.Env { - updated, err := expandArgs(lex, v, args) + updated, err := expandArgs(lex, v, args, allowArg) if err != nil { appendErr(fmt.Errorf("error performing shell expansion on env var %q for source %q: %w", k, name, err)) } @@ -618,7 +636,7 @@ func (c *TestSpec) processBuildArgs(lex *shell.Lex, args map[string]string, name for i, step := range c.Steps { for k, v := range step.Env { - updated, err := expandArgs(lex, v, args) + updated, err := expandArgs(lex, v, args, allowArg) if err != nil { appendErr(fmt.Errorf("error performing shell expansion on env var %q for source %q: %w", k, name, err)) } @@ -629,13 +647,13 @@ func (c *TestSpec) processBuildArgs(lex *shell.Lex, args map[string]string, name for i, step := range c.Steps { stdout := step.Stdout - if err := stdout.processBuildArgs(lex, args); err != nil { + if err := stdout.processBuildArgs(lex, args, allowArg); err != nil { appendErr(err) } step.Stdout = stdout stderr := step.Stderr - if err := stderr.processBuildArgs(lex, args); err != nil { + if err := stderr.processBuildArgs(lex, args, allowArg); err != nil { appendErr(err) } @@ -644,7 +662,7 @@ func (c *TestSpec) processBuildArgs(lex *shell.Lex, args map[string]string, name } for name, f := range c.Files { - if err := f.processBuildArgs(lex, args); err != nil { + if err := f.processBuildArgs(lex, args, allowArg); err != nil { appendErr(fmt.Errorf("error performing shell expansion to check output of file %s: %w", name, err)) } c.Files[name] = f @@ -653,9 +671,9 @@ func (c *TestSpec) processBuildArgs(lex *shell.Lex, args map[string]string, name return goerrors.Join(errs...) } -func (c *FileCheckOutput) processBuildArgs(lex *shell.Lex, args map[string]string) error { +func (c *FileCheckOutput) processBuildArgs(lex *shell.Lex, args map[string]string, allowArg func(string) bool) error { check := c.CheckOutput - if err := check.processBuildArgs(lex, args); err != nil { + if err := check.processBuildArgs(lex, args, allowArg); err != nil { return err } c.CheckOutput = check @@ -671,9 +689,9 @@ func (g *SourceGenerator) Validate() error { return nil } -func (s *PackageSigner) processBuildArgs(lex *shell.Lex, args map[string]string) error { +func (s *PackageSigner) processBuildArgs(lex *shell.Lex, args map[string]string, allowArg func(string) bool) error { for k, v := range s.Args { - updated, err := expandArgs(lex, v, args) + updated, err := expandArgs(lex, v, args, allowArg) if err != nil { return fmt.Errorf("error performing shell expansion on env var %q: %w", k, err) } @@ -682,15 +700,15 @@ func (s *PackageSigner) processBuildArgs(lex *shell.Lex, args map[string]string) return nil } -func (t *Target) processBuildArgs(name string, lex *shell.Lex, args map[string]string) error { +func (t *Target) processBuildArgs(name string, lex *shell.Lex, args map[string]string, allowArg func(string) bool) error { for _, tt := range t.Tests { - if err := tt.processBuildArgs(lex, args, path.Join(name, tt.Name)); err != nil { + if err := tt.processBuildArgs(lex, args, path.Join(name, tt.Name), allowArg); err != nil { return err } } if t.PackageConfig != nil { - if err := t.PackageConfig.processBuildArgs(lex, args); err != nil { + if err := t.PackageConfig.processBuildArgs(lex, args, allowArg); err != nil { return fmt.Errorf("error processing package config build args: %w", err) } } @@ -698,9 +716,9 @@ func (t *Target) processBuildArgs(name string, lex *shell.Lex, args map[string]s return nil } -func (cfg *PackageConfig) processBuildArgs(lex *shell.Lex, args map[string]string) error { +func (cfg *PackageConfig) processBuildArgs(lex *shell.Lex, args map[string]string, allowArg func(string) bool) error { if cfg.Signer != nil { - if err := cfg.Signer.processBuildArgs(lex, args); err != nil { + if err := cfg.Signer.processBuildArgs(lex, args, allowArg); err != nil { return fmt.Errorf("could not process build args for signer config: %w", err) } } diff --git a/load_test.go b/load_test.go index 5364dc210..69a0814f5 100644 --- a/load_test.go +++ b/load_test.go @@ -557,6 +557,14 @@ func TestSpec_SubstituteBuildArgs(t *testing.T) { err := spec.SubstituteArgs(env) assert.ErrorIs(t, err, errUnknownArg, "args not defined in the spec should error out") + // Now with the arg explicitly allowed as a passhtrough + err = spec.SubstituteArgs(env, func(cfg *SubstituteConfig) { + cfg.AllowArg = func(key string) bool { + return key == "FOO" + } + }) + assert.NilError(t, err) + spec.Args = map[string]string{} spec.Args["FOO"] = "" @@ -585,7 +593,6 @@ func TestSpec_SubstituteBuildArgs(t *testing.T) { } env["BAR"] = bar - assert.ErrorIs(t, err, errUnknownArg, "args not defined in the spec should error out") spec.Args["BAR"] = "" spec.Args["VAR_WITH_DEFAULT"] = argWithDefault @@ -603,6 +610,7 @@ func TestSpec_SubstituteBuildArgs(t *testing.T) { assert.Check(t, cmp.Equal(spec.Targets["t2"].PackageConfig.Signer.Args["BAR"], bar)) assert.Check(t, cmp.Equal(spec.Targets["t2"].PackageConfig.Signer.Args["WHATEVER"], argWithDefault)) assert.Check(t, cmp.Equal(spec.Targets["t2"].PackageConfig.Signer.Args["REGULAR"], plainOleValue)) + } func TestCustomRepoFillDefaults(t *testing.T) { From f5f9965448d92bec86227ac6a85d7a42575693a3 Mon Sep 17 00:00:00 2001 From: Brian Goff Date: Mon, 16 Dec 2024 18:06:07 -0800 Subject: [PATCH 3/3] windows: support building for multiple base images This allows the builder to specify a file (either in the main build context or a custom one) which contains a list of base images that the windowscross/container target should produce images for. This is useful for when you want to have one image for different versions of Windows, given that for Windows containers the container OS must match the host OS. Signed-off-by: Brian Goff --- frontend/build.go | 21 ++- frontend/gateway.go | 17 +- frontend/windows/dockerui.go | 123 ++++++++++++++ frontend/windows/handle_container.go | 241 ++++++++++++++++++++++----- test/windows_test.go | 132 +++++++++++++-- website/docs/targets.md | 39 +++++ 6 files changed, 507 insertions(+), 66 deletions(-) create mode 100644 frontend/windows/dockerui.go diff --git a/frontend/build.go b/frontend/build.go index 86a2e1f83..76a8b1e00 100644 --- a/frontend/build.go +++ b/frontend/build.go @@ -104,15 +104,7 @@ func fillPlatformArgs(prefix string, args map[string]string, platform ocispecs.P type PlatformBuildFunc func(ctx context.Context, client gwclient.Client, platform *ocispecs.Platform, spec *dalec.Spec, targetKey string) (gwclient.Reference, *dalec.DockerImageSpec, error) -// BuildWithPlatform is a helper function to build a spec with a given platform -// It takes care of looping through each tarrget platform and executing the build with the platform args substituted in the spec. -// This also deals with the docker-style multi-platform output. -func BuildWithPlatform(ctx context.Context, client gwclient.Client, f PlatformBuildFunc) (*gwclient.Result, error) { - dc, err := dockerui.NewClient(client) - if err != nil { - return nil, err - } - +func BuildWithPlatformFromUIClient(ctx context.Context, client gwclient.Client, dc *dockerui.Client, f PlatformBuildFunc) (*gwclient.Result, error) { rb, err := dc.Build(ctx, func(ctx context.Context, platform *ocispecs.Platform, idx int) (gwclient.Reference, *dalec.DockerImageSpec, *dalec.DockerImageSpec, error) { spec, err := LoadSpec(ctx, dc, platform) if err != nil { @@ -133,6 +125,17 @@ func BuildWithPlatform(ctx context.Context, client gwclient.Client, f PlatformBu return rb.Finalize() } +// BuildWithPlatform is a helper function to build a spec with a given platform +// It takes care of looping through each target platform and executing the build with the platform args substituted in the spec. +// This also deals with the docker-style multi-platform output. +func BuildWithPlatform(ctx context.Context, client gwclient.Client, f PlatformBuildFunc) (*gwclient.Result, error) { + dc, err := dockerui.NewClient(client) + if err != nil { + return nil, err + } + return BuildWithPlatformFromUIClient(ctx, client, dc, f) +} + // GetBaseImage returns an image that first checks if the client provided the // image in the build context matching the image ref. // diff --git a/frontend/gateway.go b/frontend/gateway.go index 862afb67e..415e4a8c5 100644 --- a/frontend/gateway.go +++ b/frontend/gateway.go @@ -101,12 +101,7 @@ func GetBuildArg(client gwclient.Client, k string) (string, bool) { return "", false } -func SourceOptFromClient(ctx context.Context, c gwclient.Client) (dalec.SourceOpts, error) { - dc, err := dockerui.NewClient(c) - if err != nil { - return dalec.SourceOpts{}, err - } - +func SourceOptFromUIClient(ctx context.Context, c gwclient.Client, dc *dockerui.Client) dalec.SourceOpts { return dalec.SourceOpts{ Resolver: c, Forward: ForwarderFromClient(ctx, c), @@ -125,7 +120,15 @@ func SourceOptFromClient(ctx context.Context, c gwclient.Client) (dalec.SourceOp } return st, nil }, - }, nil + } +} + +func SourceOptFromClient(ctx context.Context, c gwclient.Client) (dalec.SourceOpts, error) { + dc, err := dockerui.NewClient(c) + if err != nil { + return dalec.SourceOpts{}, err + } + return SourceOptFromUIClient(ctx, c, dc), nil } var ( diff --git a/frontend/windows/dockerui.go b/frontend/windows/dockerui.go new file mode 100644 index 000000000..8781604b1 --- /dev/null +++ b/frontend/windows/dockerui.go @@ -0,0 +1,123 @@ +package windows + +import ( + "context" + "encoding/json" + "fmt" + + "github.com/containerd/platforms" + "github.com/moby/buildkit/exporter/containerimage/exptypes" + "github.com/moby/buildkit/frontend/dockerui" + gwclient "github.com/moby/buildkit/frontend/gateway/client" + ocispecs "github.com/opencontainers/image-spec/specs-go/v1" + "github.com/pkg/errors" + "golang.org/x/sync/errgroup" +) + +// This is a copy of dockerui.Client.Build +// It has one modification: Instead of `platforms.Format` it uses `platforms.FormatAll` +// The value returned from this function is used as a map key to store build +// result references. +// When `platforms.Format` is used, the `OSVersion` field is not taken into account +// which means we end up overwriting map keys when there are multiple windows +// platform images being output but with different OSVerions. +// platforms.FormatAll takes OSVersion into account. +func dcBuild(ctx context.Context, bc *dockerui.Client, fn dockerui.BuildFunc) (*resultBuilder, error) { + res := gwclient.NewResult() + + targets := make([]*ocispecs.Platform, 0, len(bc.TargetPlatforms)) + for _, p := range bc.TargetPlatforms { + p := p + targets = append(targets, &p) + } + if len(targets) == 0 { + targets = append(targets, nil) + } + expPlatforms := &exptypes.Platforms{ + Platforms: make([]exptypes.Platform, len(targets)), + } + + eg, ctx := errgroup.WithContext(ctx) + + for i, tp := range targets { + i, tp := i, tp + eg.Go(func() error { + ref, img, baseImg, err := fn(ctx, tp, i) + if err != nil { + return err + } + + config, err := json.Marshal(img) + if err != nil { + return errors.Wrapf(err, "failed to marshal image config") + } + + var baseConfig []byte + if baseImg != nil { + baseConfig, err = json.Marshal(baseImg) + if err != nil { + return errors.Wrapf(err, "failed to marshal source image config") + } + } + + p := platforms.DefaultSpec() + if tp != nil { + p = *tp + } + + // in certain conditions we allow input platform to be extended from base image + if p.OS == "windows" && img.OS == p.OS { + if p.OSVersion == "" && img.OSVersion != "" { + p.OSVersion = img.OSVersion + } + if p.OSFeatures == nil && len(img.OSFeatures) > 0 { + p.OSFeatures = append([]string{}, img.OSFeatures...) + } + } + + p = platforms.Normalize(p) + k := platforms.FormatAll(p) + + if bc.MultiPlatformRequested { + res.AddRef(k, ref) + res.AddMeta(fmt.Sprintf("%s/%s", exptypes.ExporterImageConfigKey, k), config) + if len(baseConfig) > 0 { + res.AddMeta(fmt.Sprintf("%s/%s", exptypes.ExporterImageBaseConfigKey, k), baseConfig) + } + } else { + res.SetRef(ref) + res.AddMeta(exptypes.ExporterImageConfigKey, config) + if len(baseConfig) > 0 { + res.AddMeta(exptypes.ExporterImageBaseConfigKey, baseConfig) + } + } + expPlatforms.Platforms[i] = exptypes.Platform{ + ID: k, + Platform: p, + } + return nil + }) + } + if err := eg.Wait(); err != nil { + return nil, err + } + return &resultBuilder{ + Result: res, + expPlatforms: expPlatforms, + }, nil +} + +type resultBuilder struct { + *gwclient.Result + expPlatforms *exptypes.Platforms +} + +func (rb *resultBuilder) Finalize() (*gwclient.Result, error) { + dt, err := json.Marshal(rb.expPlatforms) + if err != nil { + return nil, err + } + rb.AddMeta(exptypes.ExporterPlatformsKey, dt) + + return rb.Result, nil +} diff --git a/frontend/windows/handle_container.go b/frontend/windows/handle_container.go index bd88a2310..bca17342a 100644 --- a/frontend/windows/handle_container.go +++ b/frontend/windows/handle_container.go @@ -6,19 +6,26 @@ import ( "fmt" "path" "runtime" + "sync" "github.com/Azure/dalec" "github.com/Azure/dalec/frontend" + "github.com/containerd/platforms" "github.com/moby/buildkit/client/llb" "github.com/moby/buildkit/client/llb/sourceresolver" + "github.com/moby/buildkit/frontend/dockerui" gwclient "github.com/moby/buildkit/frontend/gateway/client" ocispecs "github.com/opencontainers/image-spec/specs-go/v1" "github.com/pkg/errors" + "golang.org/x/sync/errgroup" ) const ( defaultBaseImage = "mcr.microsoft.com/windows/nanoserver:1809" windowsSystemDir = "/Windows/System32/" + + argBasesPathKey = "DALEC_WINDOWSCROSS_BASES_PATH" + argBasesContextKey = "DALEC_WINDOWSCROSS_BASES_CONTEXT" ) var ( @@ -32,35 +39,201 @@ var ( } ) +// ImageBases is the structure used by clients that want to specify multiple +// base images for the container build target via the `DALEC_WINDOWSCROSS_BASES_PATH` +// and `DALEC_WINDOWSCROSS_BASES_CONTEXT` build-args. +type ImageBases struct { + Refs []string `json:"refs,omitempty" yaml:"refs,omitempty"` +} + +func (ib *ImageBases) getRefs() []string { + if ib == nil { + return nil + } + return ib.Refs +} + +func (ib *ImageBases) len() int { + if ib == nil { + return 0 + } + return len(ib.Refs) +} + +func getImageBases(ctx context.Context, client gwclient.Client, sOpt dalec.SourceOpts) (*ImageBases, error) { + + bOpts := client.BuildOpts().Opts + p := bOpts["build-arg:"+argBasesPathKey] + if p == "" { + return nil, nil + } + + src := dalec.Source{ + Context: &dalec.SourceContext{Name: "context"}, + Path: p, + } + + if name := bOpts["build-arg:"+argBasesContextKey]; name != "" { + src.Context.Name = name + } + + pg := dalec.ProgressGroup("Determine base images") + st, err := src.AsMount("src", sOpt, pg) + if err != nil { + return nil, err + } + + def, err := st.Marshal(ctx, pg) + if err != nil { + return nil, errors.Wrapf(err, "marshalling state to solve for \"%s=%s\" and \"%s=%s\"", argBasesPathKey, p, argBasesContextKey, src.Context.Name) + } + + res, err := client.Solve(ctx, gwclient.SolveRequest{Definition: def.ToPB()}) + if err != nil { + return nil, errors.Wrapf(err, "solving state for \"%s=%s\" and \"%s=%s\"", argBasesPathKey, p, argBasesContextKey, src.Context.Name) + } + + ref, err := res.SingleRef() + if err != nil { + return nil, err + } + + dt, err := ref.ReadFile(ctx, gwclient.ReadRequest{Filename: p}) + if err != nil { + return nil, err + } + + var bases ImageBases + + if err := json.Unmarshal(dt, &bases); err != nil { + return nil, err + } + return &bases, nil +} + func handleContainer(ctx context.Context, client gwclient.Client) (*gwclient.Result, error) { - return frontend.BuildWithPlatform(ctx, client, func(ctx context.Context, client gwclient.Client, platform *ocispecs.Platform, spec *dalec.Spec, targetKey string) (gwclient.Reference, *dalec.DockerImageSpec, error) { - sOpt, err := frontend.SourceOptFromClient(ctx, client) + dc, err := dockerui.NewClient(client) + if err != nil { + return nil, err + } + + if len(dc.TargetPlatforms) > 1 { + return nil, fmt.Errorf("multi-platform output is not supported") + } + + sOpt := frontend.SourceOptFromUIClient(ctx, client, dc) + + bases, err := getImageBases(ctx, client, sOpt) + if err != nil { + return nil, err + } + + refs := bases.getRefs() + if len(refs) == 0 { + refs = append(refs, defaultBaseImage) + } + + eg, grpCtx := errgroup.WithContext(ctx) + cfgs := make([][]byte, len(refs)) + targets := make([]ocispecs.Platform, len(cfgs)) + + basePlatform := defaultPlatform + if len(dc.TargetPlatforms) > 0 { + basePlatform = dc.TargetPlatforms[0] + } + + for idx, ref := range refs { + idx := idx + ref := ref + eg.Go(func() error { + _, _, dt, err := client.ResolveImageConfig(grpCtx, ref, sourceresolver.Opt{ + Platform: &basePlatform, + ImageOpt: &sourceresolver.ResolveImageOpt{ + ResolveMode: dc.ImageResolveMode.String(), + }, + }) + + if err != nil { + return err + } + + var cfg dalec.DockerImageSpec + if err := json.Unmarshal(dt, &cfg); err != nil { + return errors.Wrapf(err, "image config for %s", ref) + } + + cfgs[idx] = dt + targets[idx] = cfg.Platform + + return nil + }) + } + + if err := eg.Wait(); err != nil { + return nil, err + } + + seen := make(map[string]struct{}) + for _, p := range targets { + s := platforms.FormatAll(p) + if _, ok := seen[s]; ok { + return nil, fmt.Errorf("mutiple base images provided with the same platform value") + } + seen[s] = struct{}{} + } + + dc.TargetPlatforms = targets + if len(targets) > 1 { + dc.MultiPlatformRequested = true + } + targetKey := frontend.GetTargetKey(client) + + warnBaseOverride := sync.OnceFunc(func() { + frontend.Warn(ctx, client, llb.Scratch(), "Base image defined in spec overwritten by base images context") + }) + + getBaseRef := func(idx int, spec *dalec.Spec) string { + baseRef := refs[idx] + + updated := dalec.GetBaseOutputImage(spec, targetKey) + if updated == "" { + return baseRef + } + + if bases.len() == 0 { + return updated + } + + warnBaseOverride() + return baseRef + } + + rb, err := dcBuild(ctx, dc, func(ctx context.Context, platform *ocispecs.Platform, idx int) (ref gwclient.Reference, retCfg, retBaseCfg *dalec.DockerImageSpec, retErr error) { + spec, err := frontend.LoadSpec(ctx, dc, platform, frontend.WithAllowArgs( + argBasesPathKey, + argBasesContextKey, + )) if err != nil { - return nil, nil, err + return nil, nil, nil, err } if err := validateRuntimeDeps(spec, targetKey); err != nil { - return nil, nil, fmt.Errorf("error validating windows spec: %w", err) + return nil, nil, nil, fmt.Errorf("error validating windows spec: %w", err) } pg := dalec.ProgressGroup("Build windows container: " + spec.Name) worker, err := distroConfig.Worker(sOpt, pg) if err != nil { - return nil, nil, err + return nil, nil, nil, err } bin, err := buildBinaries(ctx, spec, worker, client, sOpt, targetKey) if err != nil { - return nil, nil, fmt.Errorf("unable to build binary %w", err) + return nil, nil, nil, fmt.Errorf("unable to build binary %w", err) } - baseImgName := getBaseOutputImage(spec, targetKey, defaultBaseImage) - - if platform == nil { - platform = &defaultPlatform - } - - baseImage := llb.Image(baseImgName, llb.Platform(*platform)) + baseRef := getBaseRef(idx, spec) + baseImage := llb.Image(baseRef, llb.Platform(*platform)) out := baseImage. File(llb.Copy(bin, "/", windowsSystemDir)). @@ -68,40 +241,39 @@ func handleContainer(ctx context.Context, client gwclient.Client) (*gwclient.Res def, err := out.Marshal(ctx) if err != nil { - return nil, nil, err + return nil, nil, nil, err } res, err := client.Solve(ctx, gwclient.SolveRequest{ Definition: def.ToPB(), }) if err != nil { - return nil, nil, err + return nil, nil, nil, err } - imgRef := dalec.GetBaseOutputImage(spec, targetKey) - if imgRef == "" { - imgRef = defaultBaseImage - } - - _, _, dt, err := client.ResolveImageConfig(ctx, imgRef, sourceresolver.Opt{ - Platform: platform, - }) - if err != nil { - return nil, nil, errors.Wrap(err, "could not resolve base image config") + var baseCfg dalec.DockerImageSpec + if err := json.Unmarshal(cfgs[idx], &baseCfg); err != nil { + return nil, nil, nil, errors.Wrap(err, "error unmarshalling base image config") } + // Get a copy of the cfg so we can modify it var img dalec.DockerImageSpec - if err := json.Unmarshal(dt, &img); err != nil { - return nil, nil, errors.Wrap(err, "error unmarshalling base image config") + if err := json.Unmarshal(cfgs[idx], &img); err != nil { + return nil, nil, nil, errors.Wrap(err, "error unmarshalling base image config") } if err := dalec.BuildImageConfig(spec, targetKey, &img); err != nil { - return nil, nil, errors.Wrap(err, "error creating image config") + return nil, nil, nil, errors.Wrap(err, "error creating image config") } - ref, err := res.SingleRef() - return ref, &img, err + ref, err = res.SingleRef() + return ref, &img, &baseCfg, err }) + if err != nil { + return nil, err + } + + return rb.Finalize() } func copySymlinks(post *dalec.PostInstall) llb.StateOption { @@ -124,13 +296,4 @@ func copySymlinks(post *dalec.PostInstall) llb.StateOption { return s } - -} - -func getBaseOutputImage(spec *dalec.Spec, target, defaultBase string) string { - baseRef := defaultBase - if spec.Targets[target].Image != nil && spec.Targets[target].Image.Base != "" { - baseRef = spec.Targets[target].Image.Base - } - return baseRef } diff --git a/test/windows_test.go b/test/windows_test.go index e5c6fae1f..a61b76e5e 100644 --- a/test/windows_test.go +++ b/test/windows_test.go @@ -2,6 +2,7 @@ package test import ( "context" + "encoding/json" "errors" "fmt" "testing" @@ -9,11 +10,16 @@ import ( "github.com/Azure/dalec" "github.com/Azure/dalec/frontend/ubuntu" "github.com/Azure/dalec/frontend/windows" + "github.com/containerd/platforms" "github.com/moby/buildkit/client/llb" + "github.com/moby/buildkit/client/llb/sourceresolver" + "github.com/moby/buildkit/exporter/containerimage/exptypes" gwclient "github.com/moby/buildkit/frontend/gateway/client" moby_buildkit_v1_frontend "github.com/moby/buildkit/frontend/gateway/pb" ocispecs "github.com/opencontainers/image-spec/specs-go/v1" "golang.org/x/exp/maps" + "gotest.tools/v3/assert" + "gotest.tools/v3/assert/cmp" ) var windowsAmd64 = ocispecs.Platform{OS: "windows", Architecture: "amd64"} @@ -184,6 +190,9 @@ func testWindows(ctx context.Context, t *testing.T, tcfg targetConfig) { }) }) t.Run("container", func(t *testing.T) { + t.Parallel() + ctx := startTestSpan(ctx, t) + spec := dalec.Spec{ Name: "test-container-build", Version: "0.0.1", @@ -316,17 +325,8 @@ echo "$BAR" > bar.txt }, } - testEnv.RunTest(ctx, t, func(ctx context.Context, gwc gwclient.Client) { - sr := newSolveRequest(withSpec(ctx, t, &spec), withBuildTarget(tcfg.Container), withWindowsAmd64) - sr.Evaluate = true - res := solveT(ctx, t, gwc, sr) - - ref, err := res.SingleRef() - if err != nil { - t.Fatal(err) - } - - post := spec.GetImagePost("windowscross") + post := spec.GetImagePost("windowscross") + validateSymlinks := func(ctx context.Context, t *testing.T, ref gwclient.Reference) { for srcPath, l := range post.Symlinks { b1, err := ref.ReadFile(ctx, gwclient.ReadRequest{ Filename: srcPath, @@ -352,6 +352,116 @@ echo "$BAR" > bar.txt } } } + } + + t.Run("single-image", func(t *testing.T) { + t.Parallel() + ctx := startTestSpan(ctx, t) + + testEnv.RunTest(ctx, t, func(ctx context.Context, gwc gwclient.Client) { + sr := newSolveRequest(withSpec(ctx, t, &spec), withBuildTarget(tcfg.Container), withWindowsAmd64) + sr.Evaluate = true + res := solveT(ctx, t, gwc, sr) + + ref, err := res.SingleRef() + if err != nil { + t.Fatal(err) + } + validateSymlinks(ctx, t, ref) + }) + }) + + t.Run("multi-image", func(t *testing.T) { + t.Parallel() + ctx := startTestSpan(ctx, t) + + testEnv.RunTest(ctx, t, func(ctx context.Context, gwc gwclient.Client) { + + bases := windows.ImageBases{ + Refs: []string{ + "mcr.microsoft.com/windows/nanoserver:ltsc2022", + "mcr.microsoft.com/windows/nanoserver:1809", + }, + } + + dt, err := json.Marshal(bases) + assert.NilError(t, err) + + st := llb.Scratch().File( + llb.Mkfile("bases.json", 0o644, dt), + ) + sr := newSolveRequest( + withSpec(ctx, t, &spec), + withBuildTarget(tcfg.Container), + withWindowsAmd64, + withBuildContext(ctx, t, "multi-image", st), + withBuildArg("DALEC_WINDOWSCROSS_BASES_PATH", "bases.json"), + withBuildArg("DALEC_WINDOWSCROSS_BASES_CONTEXT", "multi-image"), + ) + sr.Evaluate = true + res := solveT(ctx, t, gwc, sr) + + var metaPlatforms exptypes.Platforms + err = json.Unmarshal(res.Metadata["refs.platforms"], &metaPlatforms) + assert.NilError(t, err) + assert.Assert(t, cmp.Len(metaPlatforms.Platforms, 2)) + + // Go through each of the base images we requested and resolve + // them so we can get the platform info + // Then validate that the platform for the base image matches the platform + // in the result platforms. + for i, ref := range bases.Refs { + actual := metaPlatforms.Platforms[i] + + _, _, dt, err = gwc.ResolveImageConfig(ctx, ref, sourceresolver.Opt{ + Platform: &windowsAmd64, + }) + assert.NilError(t, err) + + var cfg dalec.DockerImageSpec + assert.NilError(t, json.Unmarshal(dt, &cfg)) + assert.Check(t, cmp.Equal(cfg.OS, actual.Platform.OS)) + assert.Check(t, cmp.Equal(cfg.Architecture, actual.Platform.Architecture)) + assert.Check(t, cmp.Equal(cfg.OSVersion, actual.Platform.OSVersion)) + } + + // NOTE: we are not using `res.SingleRef` because we requested multiple + // refs which would cause an error in this case. + // Instead we need to look at res.Refs + assert.Assert(t, cmp.Len(res.Refs, len(metaPlatforms.Platforms))) + + for _, p := range metaPlatforms.Platforms { + ref, ok := res.Refs[platforms.FormatAll(p.Platform)] + assert.Assert(t, ok, "unepxected ref keys: %s", maps.Keys(res.Refs)) + validateSymlinks(ctx, t, ref) + } + + // This should fail since the bases have the same platform + bases = windows.ImageBases{ + Refs: []string{ + "mcr.microsoft.com/windows/nanoserver:ltsc2022", + "mcr.microsoft.com/windows/nanoserver:ltsc2022-amd64", + }, + } + + dt, err = json.Marshal(bases) + assert.NilError(t, err) + + st = llb.Scratch().File( + llb.Mkfile("bases.json", 0o644, dt), + ) + sr = newSolveRequest( + withSpec(ctx, t, &spec), + withBuildTarget(tcfg.Container), + withWindowsAmd64, + withBuildContext(ctx, t, "multi-image", st), + withBuildArg("DALEC_WINDOWSCROSS_BASES_PATH", "bases.json"), + withBuildArg("DALEC_WINDOWSCROSS_BASES_CONTEXT", "multi-image"), + ) + sr.Evaluate = true + _, err = gwc.Solve(ctx, sr) + assert.ErrorContains(t, err, "mutiple base images provided with the same") + }) }) }) diff --git a/website/docs/targets.md b/website/docs/targets.md index ed58eb95c..c42741102 100644 --- a/website/docs/targets.md +++ b/website/docs/targets.md @@ -144,3 +144,42 @@ This works the same way in the `azlinux3`: i. `--build-context mcr.microsoft.com/azurelinux/base/core:3.0=` 2. A build context named `dalec-mariner2-worker` i. `--build-context dalec-azlinux3-worker=` + +#### Windows + +For Windows containers, typically the container image OS needs to match +the Windows host OS. + +You can use DALEC to create a single multi-platform image with the different +Windows versions you want to use. +Normally you would specify a single base image in the DALEC spec's image config, +however this is not sufficient to accomplish this task. + +With DALEC you can pass in a build-arg `DALEC_WINDOWSCROSS_BASES_PATH` the value +of which should be the path to a file containing json with the following +structure to the `windowscross/container` build target: + + +```json +{ + "refs": [ + "mcr.microsoft.com/windows/nanoserver:1809", + "mcr.microsoft.com/windows/nanoserver:ltsc2022" + ] +} +``` + +:::note +Values in the "refs" field can be any Windows image. + +You can provide any number of images here, however each image must have a +different value for the `os.version` field in the image config's platform. +If there are images with the same platform values the build will fail. +::: + +You can also provide this file in a named build context, but you must still +specifiy the above mentioned build-arg so that DALEC knows how to find the file +in that named context. +You can tell DALEC to use a named context by providing the name in a build-arg +`DALEC_WINDOWSCROSS_BASES_CONTEXT` +