From 4b98860b3814f0f1f74f62a272337537acd6c3ec Mon Sep 17 00:00:00 2001 From: Brian Goff Date: Wed, 29 May 2024 00:05:49 +0000 Subject: [PATCH] Add support for passing build args to signers I considered extending this to allow this for any kind of frontend (and as such sticking it into the `dalec.Frontend` type), however there really shouldn't be different vars passed to target frontends. Hence why I am restricting this to just signers for now. If we decide to later we can add this more generically, however it will be much more difficult to take it away if we decide it was a bad idea. Signed-off-by: Brian Goff --- docs/spec.schema.json | 30 +++++++++- frontend/request.go | 20 ++++++- helpers.go | 2 +- load.go | 55 +++++++++++++++-- load_test.go | 71 ++++++++++++++++++++++ spec.go | 9 ++- test/azlinux_test.go | 74 ++++++++++++++--------- test/fixtures/signer/main.go | 13 ++++ test/signing_test.go | 6 ++ test/windows_test.go | 6 +- website/docs/signing.md | 111 +++++++++++++++++++++++++++++++++++ 11 files changed, 356 insertions(+), 41 deletions(-) diff --git a/docs/spec.schema.json b/docs/spec.schema.json index 2d0f08ca7..b07c15cb0 100644 --- a/docs/spec.schema.json +++ b/docs/spec.schema.json @@ -426,7 +426,7 @@ "PackageConfig": { "properties": { "signer": { - "$ref": "#/$defs/Frontend", + "$ref": "#/$defs/PackageSigner", "description": "Signer is the configuration to use for signing packages" } }, @@ -478,6 +478,34 @@ "type": "object", "description": "PackageDependencies is a list of dependencies for a package." }, + "PackageSigner": { + "properties": { + "image": { + "type": "string", + "description": "Image specifies the frontend image to forward the build to.\nThis can be left unspecified *if* the original frontend has builtin support for the distro.\n\nIf the original frontend does not have builtin support for the distro, this must be specified or the build will fail.\nIf this is specified then it MUST be used.", + "examples": [ + "docker.io/my/frontend:latest" + ] + }, + "cmdline": { + "type": "string", + "description": "CmdLine is the command line to use to forward the build to the frontend.\nBy default the frontend image's entrypoint/cmd is used." + }, + "args": { + "additionalProperties": { + "type": "string" + }, + "type": "object", + "description": "Args are passed along to the signer frontend as build args" + } + }, + "additionalProperties": false, + "type": "object", + "required": [ + "image" + ], + "description": "PackageSigner is the configuration for defining how to sign a package" + }, "PatchSpec": { "properties": { "source": { diff --git a/frontend/request.go b/frontend/request.go index fdd193949..23cfe825b 100644 --- a/frontend/request.go +++ b/frontend/request.go @@ -69,6 +69,22 @@ func withTarget(t string) solveRequestOpt { } } +func withBuildArgs(args map[string]string) solveRequestOpt { + return func(req *gwclient.SolveRequest) error { + if len(args) == 0 { + return nil + } + + if req.FrontendOpt == nil { + req.FrontendOpt = make(map[string]string) + } + for k, v := range args { + req.FrontendOpt["build-arg:"+k] = v + } + return nil + } +} + func toDockerfile(ctx context.Context, bctx llb.State, dt []byte, spec *dalec.SourceBuild, opts ...llb.ConstraintsOpt) solveRequestOpt { return func(req *gwclient.SolveRequest) error { req.Frontend = "dockerfile.v0" @@ -118,7 +134,7 @@ func marshalDockerfile(ctx context.Context, dt []byte, opts ...llb.ConstraintsOp return st.Marshal(ctx) } -func ForwardToSigner(ctx context.Context, client gwclient.Client, cfg *dalec.Frontend, s llb.State) (llb.State, error) { +func ForwardToSigner(ctx context.Context, client gwclient.Client, cfg *dalec.PackageSigner, s llb.State) (llb.State, error) { const ( sourceKey = "source" contextKey = "context" @@ -127,7 +143,7 @@ func ForwardToSigner(ctx context.Context, client gwclient.Client, cfg *dalec.Fro opts := client.BuildOpts().Opts - req, err := newSolveRequest(toFrontend(cfg)) + req, err := newSolveRequest(toFrontend(cfg.Frontend), withBuildArgs(cfg.Args)) if err != nil { return llb.Scratch(), err } diff --git a/helpers.go b/helpers.go index a5b082c6b..7d4af1a65 100644 --- a/helpers.go +++ b/helpers.go @@ -340,7 +340,7 @@ func (s *Spec) GetSymlinks(target string) map[string]SymlinkTarget { return lm } -func (s *Spec) GetSigner(targetKey string) (*Frontend, bool) { +func (s *Spec) GetSigner(targetKey string) (*PackageSigner, bool) { if s.Targets != nil { if t, ok := s.Targets[targetKey]; ok && hasValidSigner(t.PackageConfig) { return t.PackageConfig.Signer, true diff --git a/load.go b/load.go index 630cdab07..97a1318d3 100644 --- a/load.go +++ b/load.go @@ -192,6 +192,8 @@ func (s *Source) validate(failContext ...string) (retErr error) { return retErr } +var errUnknownArg = errors.New("unknown arg") + func (s *Spec) SubstituteArgs(env map[string]string) error { lex := shell.NewLex('\\') @@ -202,7 +204,7 @@ func (s *Spec) SubstituteArgs(env map[string]string) error { for k, v := range env { if _, ok := args[k]; !ok { if !knownArg(k) { - return fmt.Errorf("unknown arg %q", k) + return fmt.Errorf("%w: %q", errUnknownArg, k) } // if the build arg isn't present in args by opt-in, skip @@ -259,11 +261,15 @@ func (s *Spec) SubstituteArgs(env map[string]string) error { } } - for name, target := range s.Targets { - for _, t := range target.Tests { - if err := t.processBuildArgs(lex, args, path.Join(name, t.Name)); err != nil { - return err - } + for name, t := range s.Targets { + if err := t.processBuildArgs(name, lex, args); err != nil { + return 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 { + return fmt.Errorf("could not process build args for base spec package config: %w", err) } } @@ -496,3 +502,40 @@ func (g *SourceGenerator) Validate() error { } return nil } + +func (s *PackageSigner) processBuildArgs(lex *shell.Lex, args map[string]string) error { + for k, v := range s.Args { + updated, err := lex.ProcessWordWithMap(v, args) + if err != nil { + return fmt.Errorf("error performing shell expansion on env var %q: %w", k, err) + } + s.Args[k] = updated + } + return nil +} + +func (t *Target) processBuildArgs(name string, lex *shell.Lex, args map[string]string) error { + for _, tt := range t.Tests { + if err := tt.processBuildArgs(lex, args, path.Join(name, tt.Name)); err != nil { + return err + } + } + + if t.PackageConfig != nil { + if err := t.PackageConfig.processBuildArgs(lex, args); err != nil { + return fmt.Errorf("error processing package config build args: %w", err) + } + } + + return nil +} + +func (cfg *PackageConfig) processBuildArgs(lex *shell.Lex, args map[string]string) error { + if cfg.Signer != nil { + if err := cfg.Signer.processBuildArgs(lex, args); err != nil { + return fmt.Errorf("could not process build args for signer config: %w", err) + } + } + + return nil +} diff --git a/load_test.go b/load_test.go index 9de99e61b..445b5efa8 100644 --- a/load_test.go +++ b/load_test.go @@ -5,9 +5,13 @@ import ( "encoding/json" "errors" "fmt" + "maps" "os" "reflect" "testing" + + "gotest.tools/v3/assert" + "gotest.tools/v3/assert/cmp" ) //go:embed test/fixtures/unmarshall/source-inline.yml @@ -470,3 +474,70 @@ sources: } }) } + +func TestSpec_SubstituteBuildArgs(t *testing.T) { + spec := &Spec{} + assert.NilError(t, spec.SubstituteArgs(nil)) + + env := map[string]string{} + assert.NilError(t, spec.SubstituteArgs(env)) + + // some values we'll be using throughout the test + const ( + foo = "foo" + bar = "bar" + argWithDefault = "some default value" + plainOleValue = "some plain old value" + ) + + env["FOO"] = foo + err := spec.SubstituteArgs(env) + assert.ErrorIs(t, err, errUnknownArg, "args not defined in the spec should error out") + + spec.Args = map[string]string{} + + spec.Args["FOO"] = "" + assert.NilError(t, spec.SubstituteArgs(env)) + + pairs := map[string]string{ + "FOO": "$FOO", + "BAR": "$BAR", + "WHATEVER": "$VAR_WITH_DEFAULT", + "REGULAR": plainOleValue, + } + spec.PackageConfig = &PackageConfig{ + Signer: &PackageSigner{ + Args: maps.Clone(pairs), + }, + } + spec.Targets = map[string]Target{ + "t1": {}, // nil signer + "t2": { + PackageConfig: &PackageConfig{ + Signer: &PackageSigner{ + Args: maps.Clone(pairs), + }, + }, + }, + } + + 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 + + assert.NilError(t, spec.SubstituteArgs(env)) + + // Base package config + assert.Check(t, cmp.Equal(spec.PackageConfig.Signer.Args["FOO"], foo)) + assert.Check(t, cmp.Equal(spec.PackageConfig.Signer.Args["BAR"], bar)) + assert.Check(t, cmp.Equal(spec.PackageConfig.Signer.Args["WHATEVER"], argWithDefault)) + assert.Check(t, cmp.Equal(spec.PackageConfig.Signer.Args["REGULAR"], plainOleValue)) + + // targets + assert.Check(t, cmp.Nil(spec.Targets["t1"].Frontend)) + 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)) +} diff --git a/spec.go b/spec.go index 4d83e5c4e..eaedbbc4e 100644 --- a/spec.go +++ b/spec.go @@ -523,10 +523,17 @@ type Target struct { PackageConfig *PackageConfig `yaml:"package_config,omitempty" json:"package_config,omitempty"` } +// PackageSigner is the configuration for defining how to sign a package +type PackageSigner struct { + *Frontend `yaml:",inline" json:",inline"` + // Args are passed along to the signer frontend as build args + Args map[string]string `yaml:"args,omitempty" json:"args,omitempty"` +} + // PackageConfig encapsulates the configuration for artifact targets type PackageConfig struct { // Signer is the configuration to use for signing packages - Signer *Frontend `yaml:"signer,omitempty" json:"signer,omitempty"` + Signer *PackageSigner `yaml:"signer,omitempty" json:"signer,omitempty"` } // TestSpec is used to execute tests against a container with the package installed in it. diff --git a/test/azlinux_test.go b/test/azlinux_test.go index ab0d9418f..6bf399cad 100644 --- a/test/azlinux_test.go +++ b/test/azlinux_test.go @@ -303,42 +303,60 @@ echo "$BAR" > bar.txt } t.Run("test signing", func(t *testing.T) { - t.Parallel() - spec := dalec.Spec{ - Name: "foo", - Version: "v0.0.1", - Description: "foo bar baz", - Website: "https://foo.bar.baz", - Revision: "1", - PackageConfig: &dalec.PackageConfig{ - Signer: &dalec.Frontend{ - Image: phonySignerRef, - }, - }, - Sources: map[string]dalec.Source{ - "foo": { - Inline: &dalec.SourceInline{ - File: &dalec.SourceInlineFile{ - Contents: "#!/usr/bin/env bash\necho \"hello, world!\"\n", + newSpec := func() *dalec.Spec { + return &dalec.Spec{ + Name: "foo", + Version: "v0.0.1", + Description: "foo bar baz", + Website: "https://foo.bar.baz", + Revision: "1", + PackageConfig: &dalec.PackageConfig{ + Signer: &dalec.PackageSigner{ + Frontend: &dalec.Frontend{ + Image: phonySignerRef, + }, + }, + }, + Sources: map[string]dalec.Source{ + "foo": { + Inline: &dalec.SourceInline{ + File: &dalec.SourceInlineFile{ + Contents: "#!/usr/bin/env bash\necho \"hello, world!\"\n", + }, }, }, }, - }, - Build: dalec.ArtifactBuild{ - Steps: []dalec.BuildStep{ - { - Command: "/bin/true", + Build: dalec.ArtifactBuild{ + Steps: []dalec.BuildStep{ + { + Command: "/bin/true", + }, }, }, - }, - Artifacts: dalec.Artifacts{ - Binaries: map[string]dalec.ArtifactConfig{ - "foo": {}, + Artifacts: dalec.Artifacts{ + Binaries: map[string]dalec.ArtifactConfig{ + "foo": {}, + }, }, - }, + } } - runTest(t, distroSigningTest(t, &spec, signTarget)) + t.Run("no args", func(t *testing.T) { + t.Parallel() + spec := newSpec() + runTest(t, distroSigningTest(t, spec, signTarget)) + }) + + t.Run("with args", func(t *testing.T) { + t.Parallel() + + spec := newSpec() + spec.PackageConfig.Signer.Args = map[string]string{ + "HELLO": "world", + "FOO": "bar", + } + runTest(t, distroSigningTest(t, spec, signTarget)) + }) }) t.Run("test systemd unit", func(t *testing.T) { diff --git a/test/fixtures/signer/main.go b/test/fixtures/signer/main.go index feccc3200..fb4b1c46a 100644 --- a/test/fixtures/signer/main.go +++ b/test/fixtures/signer/main.go @@ -74,6 +74,19 @@ func main() { File(llb.Mkfile("/target", 0o600, []byte(target))). File(llb.Mkfile("/config.json", 0o600, configBytes)) + // For any build-arg seen, write a file to /env/ with the contents + // being the value of the arg. + for k, v := range c.BuildOpts().Opts { + _, key, ok := strings.Cut(k, "build-arg:") + if !ok { + // not a build arg + continue + } + output = output. + File(llb.Mkdir("/env", 0o755)). + File(llb.Mkfile("/env/"+key, 0o600, []byte(v))) + } + def, err := output.Marshal(ctx) if err != nil { return nil, err diff --git a/test/signing_test.go b/test/signing_test.go index 3297f46ff..99e0bc02f 100644 --- a/test/signing_test.go +++ b/test/signing_test.go @@ -8,6 +8,7 @@ import ( "github.com/Azure/dalec" gwclient "github.com/moby/buildkit/frontend/gateway/client" + "github.com/stretchr/testify/assert" ) func distroSigningTest(t *testing.T, spec *dalec.Spec, buildTarget string) func(ctx context.Context, gwc gwclient.Client) (*gwclient.Result, error) { @@ -31,6 +32,11 @@ func distroSigningTest(t *testing.T, spec *dalec.Spec, buildTarget string) func( t.Fatal(fmt.Errorf("configuration incorrect")) } + for k, v := range spec.PackageConfig.Signer.Args { + dt := readFile(ctx, t, "/env/"+k, res) + assert.Equal(t, v, string(dt)) + } + return gwclient.NewResult(), nil } } diff --git a/test/windows_test.go b/test/windows_test.go index d24adf321..a301e1e22 100644 --- a/test/windows_test.go +++ b/test/windows_test.go @@ -276,8 +276,10 @@ echo "$BAR" > bar.txt Targets: map[string]dalec.Target{ "windowscross": { PackageConfig: &dalec.PackageConfig{ - Signer: &dalec.Frontend{ - Image: phonySignerRef, + Signer: &dalec.PackageSigner{ + Frontend: &dalec.Frontend{ + Image: phonySignerRef, + }, }, }, }, diff --git a/website/docs/signing.md b/website/docs/signing.md index 1c781c921..29b006a3b 100644 --- a/website/docs/signing.md +++ b/website/docs/signing.md @@ -11,6 +11,7 @@ tagged, the following can be added to the spec to trigger the signing operation: ```yaml +name: my-package targets: # Distro specific build requirements mariner2: package_config: @@ -42,3 +43,113 @@ provided as the build context. identical to the input `llb.State` in every way *except* that the desired artifacts will be signed. +A signer can also be configured at the root of the spec if there is no target +specific customization required or you are only building for one target. + +Example: + +```yaml +name: my-package +package_config: + signer: + image: "ref/to/signing:image" + cmdline: "/signer" +``` + +## Build Time customization + +Signing artifacts may require passing through build-time customizations. +This can be done through 3 mechanisms: + +1. [Secrets](#secrets) +2. [Named contexts](#named-contexts) +3. [Build arguments](#build-arguments) + +With all methods of build-time customization, the signer needs to be coded +such that it is going to consume the customizations that are passed in, as such +all such customizations are signer specific. + +### Secrets + +Secrets are passed through from the client (such as the docker CLI or buildx). +These secrets are always available to the signer. +see Docker's [secrets](https://docs.docker.com/build/building/secrets/) +documentation for more details on on how secrets can be passed into a build +using the docker CLI. + +*Note*: The docker documentation is using Dockerfiles in their examples which +are irrelevant for Dalec signing, however the CLI examples for how to pass in +those secrets is useful. + +No changes to the spec yaml are required to use secrets with a signer, except +that the signer itself needs to be setup to consume the secret(s). + +### Named Contexts + +Named contexts are passed into the build by the client. All named contexts are +available to the signer. + +A named context is just like a regular +[build context](https://docs.docker.com/build/building/context/) except that it +is given a custom name where as the regular build context is specifically named +`context`. In the scope of Dalec signing, the regular build context is the +packages that Dalec is giving to the signer to sign. +A named context can be used to provide extra data or configuration to the signer. + +Example usage with Docker: + +```console +$ docker build --build-context my-signing-config=./signing-config-dir ... +``` + +Here `my-singing-config` is the name you want to give to the context which the +signer may use to pull in the context. The `./signing-config-dir` is the data +being given as the context, in this case a local directory. This could be a +directory, a git ref, an HTTP url, etc. See the linked docker build context +documentation above for more details on what can be specified. + +Multiple named contexts may be provided. + +No changes to the spec yaml are required to use named contexts with a signer, +except that the signer itself needs to be setup to consume the named +context(s). + +### Build Arguments + +Buid arguments are key/value pairs that can be supplied in the yaml spec which +will be forwarded to the signer. + +Taking the original example above we can add build by adding an `args` with +a string-to-string mapping like so: + +```yaml +targets: # Distro specific build requirements + mariner2: + package_config: + signer: + image: "ref/to/signing:image" + cmdline: "/signer" + args: + SOME_KEY: SOME_VALUE + SOME_OTHER_KEY: SOME_OTHER_VALUE +``` + +The values of these arguments can also be taken from the client using variable +substitution like in other parts of the spec. +To use variable substituion, the args must be declared at the root of the spec: + +```yaml +args: + SOME_SIGNING_ARG: "" + SOME_OTHER_SIGNING_ARG: "default_value" + +targets: # Distro specific build requirements + mariner2: + package_config: + signer: + image: "ref/to/signing:image" + cmdline: "/signer" + args: + SOME_KEY: "${SOME_SIGNING_ARG}" + SOME_OTHER_KEY: "${SOME_OTHER_SIGNING_ARG}" +```