Send client warning when signing is disabled (#322)

Signed-off-by: Brian Goff <[email protected]>

frontend/gateway.go

@@ -10,6 +10,8 @@ import (
 	"github.com/moby/buildkit/frontend/dockerui"
 	gwclient "github.com/moby/buildkit/frontend/gateway/client"
 	"github.com/moby/buildkit/solver/pb"
+	"github.com/moby/buildkit/util/bklog"
+	"github.com/opencontainers/go-digest"
 	"github.com/pkg/errors"
 )
 
@@ -213,3 +215,27 @@ type BuildOpstGetter interface {
 func GetTargetKey(client BuildOpstGetter) string {
 	return client.BuildOpts().Opts[keyTopLevelTarget]
 }
+
+// Warn sends a warning to the client for the provided state.
+func Warn(ctx context.Context, client gwclient.Client, st llb.State, msg string) {
+	// Note: This will attempt to marshal the state to get its digest for metadata
+	// on the warning message, but it is not required to actually write the message.
+	// For this reason we can continue on error.
+
+	def, err := st.Marshal(ctx)
+	if err != nil {
+		bklog.G(ctx).WithError(err).WithField("warn", msg).Warn("Error marshalling state for outputing warning message")
+	}
+
+	var dgst digest.Digest
+	if def != nil {
+		dgst, err = def.Head()
+		if err != nil {
+			bklog.G(ctx).WithError(err).WithField("warn", msg).Warn("Could not get state digest for outputing warning message")
+		}
+	}
+
+	if err := client.Warn(ctx, dgst, msg, gwclient.WarnOpts{}); err != nil {
+		bklog.G(ctx).WithError(err).WithField("warn", msg).Warn("Error writing warning message")
+	}
+}

frontend/request.go

@@ -15,6 +15,8 @@ import (
 	"github.com/pkg/errors"
 )
 
+const keySkipSigningArg = "DALEC_SKIP_SIGNING"
+
 type solveRequestOpt func(*gwclient.SolveRequest) error
 
 func newSolveRequest(opts ...solveRequestOpt) (gwclient.SolveRequest, error) {
@@ -137,6 +139,7 @@ func marshalDockerfile(ctx context.Context, dt []byte, opts ...llb.ConstraintsOp
 
 func MaybeSign(ctx context.Context, client gwclient.Client, st llb.State, spec *dalec.Spec, targetKey string) (llb.State, error) {
 	if signingDisabled(client) {
+		Warn(ctx, client, st, "Signing disabled by build-arg "+keySkipSigningArg)
 		return st, nil
 	}
 
@@ -155,7 +158,7 @@ func MaybeSign(ctx context.Context, client gwclient.Client, st llb.State, spec *
 
 func signingDisabled(client gwclient.Client) bool {
 	bopts := client.BuildOpts().Opts
-	v, ok := bopts["build-arg:DALEC_SKIP_SIGNING"]
+	v, ok := bopts["build-arg:"+keySkipSigningArg]
 	if !ok {
 		return false
 	}

test/azlinux_test.go

@@ -6,13 +6,16 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
+	"strings"
 	"testing"
 
 	"github.com/Azure/dalec"
 	"github.com/Azure/dalec/test/testenv"
+	"github.com/moby/buildkit/client"
 	"github.com/moby/buildkit/client/llb"
 	gwclient "github.com/moby/buildkit/frontend/gateway/client"
 	moby_buildkit_v1_frontend "github.com/moby/buildkit/frontend/gateway/pb"
+	"gotest.tools/v3/assert"
 )
 
 func TestMariner2(t *testing.T) {
@@ -374,10 +377,10 @@ echo "$BAR" > bar.txt
 		})
 	})
 
-	runTest := func(t *testing.T, f testenv.TestFunc) {
+	runTest := func(t *testing.T, f testenv.TestFunc, opts ...testenv.TestRunnerOpt) {
 		t.Helper()
 		ctx := startTestSpan(baseCtx, t)
-		testEnv.RunTest(ctx, t, f)
+		testEnv.RunTest(ctx, t, f, opts...)
 	}
 
 	t.Run("test signing", func(t *testing.T) {
@@ -441,7 +444,20 @@ echo "$BAR" > bar.txt
 			t.Parallel()
 
 			spec := newSpec()
-			runTest(t, distroSkipSigningTest(t, spec, testConfig.SignTarget))
+			var found bool
+			handleStatus := func(status *client.SolveStatus) {
+				if found {
+					return
+				}
+				for _, w := range status.Warnings {
+					if strings.Contains(string(w.Short), "Signing disabled by build-arg") {
+						found = true
+						return
+					}
+				}
+			}
+			runTest(t, distroSkipSigningTest(t, spec, testConfig.SignTarget), testenv.WithSolveStatusFn(handleStatus))
+			assert.Assert(t, found, "Signing disabled warning message not emitted")
 		})
 	})
 

test/testenv/buildx.go

@@ -237,13 +237,56 @@ func withResolveLocal(so *client.SolveOpt) {
 
 type TestFunc func(context.Context, gwclient.Client)
 
-func (b *BuildxEnv) RunTest(ctx context.Context, t *testing.T, f TestFunc) {
+type TestRunnerConfig struct {
+	// SolveStatusFn replaces the builtin status logger with a custom implementation.
+	// This is useful particularly if you need to inspect the solve statuses.
+	SolveStatusFn func(*client.SolveStatus)
+}
+
+type TestRunnerOpt func(*TestRunnerConfig)
+
+// SolveStatus is convenience wrapper for [client.SolveStatus] to help disambiguate
+// imports of the [client] package.
+type SolveStatus = client.SolveStatus
+
+func WithSolveStatusFn(f func(*SolveStatus)) TestRunnerOpt {
+	return func(cfg *TestRunnerConfig) {
+		cfg.SolveStatusFn = f
+	}
+}
+
+func (b *BuildxEnv) RunTest(ctx context.Context, t *testing.T, f TestFunc, opts ...TestRunnerOpt) {
+	var cfg TestRunnerConfig
+
+	for _, o := range opts {
+		o(&cfg)
+	}
+
 	c, err := b.Buildkit(ctx)
 	if err != nil {
 		t.Fatalf("%+v", err)
 	}
 
-	ch, done := displaySolveStatus(ctx, t)
+	var (
+		ch   chan *client.SolveStatus
+		done <-chan struct{}
+	)
+
+	if cfg.SolveStatusFn != nil {
+		chDone := make(chan struct{})
+
+		ch = make(chan *client.SolveStatus, 1)
+		done = chDone
+		go func() {
+			defer close(chDone)
+
+			for msg := range ch {
+				cfg.SolveStatusFn(msg)
+			}
+		}()
+	} else {
+		ch, done = displaySolveStatus(ctx, t)
+	}
 
 	var so client.SolveOpt
 	withProjectRoot(t, &so)
@@ -261,13 +304,14 @@ func (b *BuildxEnv) RunTest(ctx context.Context, t *testing.T, f TestFunc) {
 		f(ctx, gwc)
 		return gwclient.NewResult(), nil
 	}, ch)
-	if err != nil {
-		t.Fatal(err)
-	}
 
 	// Make sure the display goroutine has finished.
 	// Ensures there's no test output after the test has finished (which the test runner will complain about)
 	<-done
+
+	if err != nil {
+		t.Fatal(err)
+	}
 }
 
 // clientForceDalecWithInput is a gwclient.Client that forces the solve request to use the main dalec frontend.

test/windows_test.go

@@ -12,6 +12,7 @@ import (
 	"github.com/moby/buildkit/client/llb"
 	gwclient "github.com/moby/buildkit/frontend/gateway/client"
 	moby_buildkit_v1_frontend "github.com/moby/buildkit/frontend/gateway/pb"
+	"gotest.tools/v3/assert"
 )
 
 func TestWindows(t *testing.T) {
@@ -258,10 +259,10 @@ echo "$BAR" > bar.txt
 		})
 	})
 
-	runTest := func(t *testing.T, f testenv.TestFunc) {
+	runTest := func(t *testing.T, f testenv.TestFunc, opts ...testenv.TestRunnerOpt) {
 		t.Helper()
 		ctx := startTestSpan(baseCtx, t)
-		testEnv.RunTest(ctx, t, f)
+		testEnv.RunTest(ctx, t, f, opts...)
 	}
 
 	t.Run("test windows signing", func(t *testing.T) {
@@ -343,6 +344,18 @@ echo "$BAR" > bar.txt
 
 	t.Run("test skipping windows signing", func(t *testing.T) {
 		t.Parallel()
+
+		var found bool
+		handleStatus := func(status *testenv.SolveStatus) {
+
+			for _, w := range status.Warnings {
+				if strings.Contains(string(w.Short), "Signing disabled by build-arg") {
+					found = true
+					return
+				}
+			}
+		}
+
 		runTest(t, func(ctx context.Context, gwc gwclient.Client) {
 			spec := fillMetadata("foo", &dalec.Spec{
 				Targets: map[string]dalec.Target{
@@ -412,7 +425,8 @@ echo "$BAR" > bar.txt
 			if _, err = maybeReadFile(ctx, "/config.json", res); err == nil {
 				t.Fatalf("signing took place even though signing was disabled")
 			}
-		})
+		}, testenv.WithSolveStatusFn(handleStatus))
+		assert.Assert(t, found, "Signing disabled warning message not emitted")
 	})
 
 	t.Run("go module", func(t *testing.T) {