From 1271e8ca9230c3f105385e3139072a82aff146b8 Mon Sep 17 00:00:00 2001 From: Sascha Grunert Date: Thu, 27 Jul 2023 11:41:11 +0200 Subject: [PATCH] Deflake base profiles OCI registry e2e tests Signed-off-by: Sascha Grunert --- cmd/spoc/main.go | 7 ++++ internal/pkg/artifact/artifact.go | 23 ++++++----- internal/pkg/artifact/artifact_test.go | 2 +- internal/pkg/cli/puller/consts.go | 3 ++ internal/pkg/cli/puller/impl.go | 10 +++-- internal/pkg/cli/puller/options.go | 18 ++++++--- internal/pkg/cli/puller/options_test.go | 38 ++++++++++++++++--- internal/pkg/cli/puller/puller.go | 1 + .../pkg/cli/puller/pullerfakes/fake_impl.go | 18 +++++---- internal/pkg/daemon/seccompprofile/impl.go | 6 +-- .../daemon/seccompprofile/seccompprofile.go | 2 +- .../seccompprofilefakes/fake_impl.go | 18 +++++---- internal/pkg/manager/spod/bindata/spod.go | 2 +- internal/pkg/manager/spod/bindata/webhook.go | 2 +- test/tc_base_profiles_oci_test.go | 2 + 15 files changed, 104 insertions(+), 48 deletions(-) diff --git a/cmd/spoc/main.go b/cmd/spoc/main.go index 5259f19f9e..b0c4cf4720 100644 --- a/cmd/spoc/main.go +++ b/cmd/spoc/main.go @@ -161,6 +161,13 @@ func main() { Aliases: []string{"p"}, Usage: "the platform to be used in format: os[/arch][/variant][:os_version]", }, + &cli.BoolFlag{ + Name: puller.FlagVerifySignature, + Aliases: []string{"s"}, + EnvVars: []string{"VERIFY_SIGNATURE"}, + Usage: "verify the signature of the artifact", + Value: true, + }, }, }, ) diff --git a/internal/pkg/artifact/artifact.go b/internal/pkg/artifact/artifact.go index 73aeea15e1..08f25e5a9d 100644 --- a/internal/pkg/artifact/artifact.go +++ b/internal/pkg/artifact/artifact.go @@ -242,20 +242,23 @@ func (a *Artifact) Pull( c context.Context, from, username, password string, platform *v1.Platform, + verifySignature bool, ) (*PullResult, error) { ctx, cancel := context.WithTimeout(c, defaultTimeout) defer cancel() - a.logger.Info("Verifying signature") - const all = ".*" - v := verify.VerifyCommand{ - CertVerifyOptions: options.CertVerifyOptions{ - CertIdentityRegexp: all, - CertOidcIssuerRegexp: all, - }, - } - if err := a.VerifyCmd(ctx, v, from); err != nil { - return nil, fmt.Errorf("verify signature: %w", err) + if verifySignature { + a.logger.Info("Verifying signature") + const all = ".*" + v := verify.VerifyCommand{ + CertVerifyOptions: options.CertVerifyOptions{ + CertIdentityRegexp: all, + CertOidcIssuerRegexp: all, + }, + } + if err := a.VerifyCmd(ctx, v, from); err != nil { + return nil, fmt.Errorf("verify signature: %w", err) + } } dir, err := a.MkdirTemp("", "pull-") diff --git a/internal/pkg/artifact/artifact_test.go b/internal/pkg/artifact/artifact_test.go index 2e7f8a0ea1..c7e352058d 100644 --- a/internal/pkg/artifact/artifact_test.go +++ b/internal/pkg/artifact/artifact_test.go @@ -394,7 +394,7 @@ func TestPull(t *testing.T) { sut := New(logr.Discard()) sut.impl = mock - res, err := sut.Pull(context.Background(), "", "foo", "bar", nil) + res, err := sut.Pull(context.Background(), "", "foo", "bar", nil, true) assert(res, err) }) } diff --git a/internal/pkg/cli/puller/consts.go b/internal/pkg/cli/puller/consts.go index 523da167cf..5eded05b0d 100644 --- a/internal/pkg/cli/puller/consts.go +++ b/internal/pkg/cli/puller/consts.go @@ -31,4 +31,7 @@ const ( // FlagPlatform is the flag for defining the platform. FlagPlatform string = "platform" + + // FlagVerifySignature is the flag for verifying the signature on pull. + FlagVerifySignature string = "verify-signature" ) diff --git a/internal/pkg/cli/puller/impl.go b/internal/pkg/cli/puller/impl.go index 3eea8328c8..a77ef08008 100644 --- a/internal/pkg/cli/puller/impl.go +++ b/internal/pkg/cli/puller/impl.go @@ -32,12 +32,16 @@ type defaultImpl struct{} //go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 -generate -header ../../../../hack/boilerplate/boilerplate.generatego.txt //counterfeiter:generate . impl type impl interface { - Pull(string, string, string, *v1.Platform) (*artifact.PullResult, error) + Pull(string, string, string, *v1.Platform, bool) (*artifact.PullResult, error) WriteFile(string, []byte, os.FileMode) error } -func (*defaultImpl) Pull(from, username, password string, platform *v1.Platform) (*artifact.PullResult, error) { - return artifact.New(logr.New(&cli.LogSink{})).Pull(context.Background(), from, username, password, platform) +func (*defaultImpl) Pull( + from, username, password string, platform *v1.Platform, verifySignature bool, +) (*artifact.PullResult, error) { + return artifact.New(logr.New(&cli.LogSink{})).Pull( + context.Background(), from, username, password, platform, verifySignature, + ) } func (*defaultImpl) WriteFile(name string, data []byte, perm os.FileMode) error { diff --git a/internal/pkg/cli/puller/options.go b/internal/pkg/cli/puller/options.go index 6feb1118cd..2d7ba669ef 100644 --- a/internal/pkg/cli/puller/options.go +++ b/internal/pkg/cli/puller/options.go @@ -29,17 +29,19 @@ import ( // Options define all possible options for the puller. type Options struct { - pullFrom string - outputFile string - username string - password string - platform *v1.Platform + pullFrom string + outputFile string + username string + password string + platform *v1.Platform + verifySignature bool } // Default returns a default options instance. func Default() *Options { return &Options{ - outputFile: DefaultOutputFile, + outputFile: DefaultOutputFile, + verifySignature: true, } } @@ -64,6 +66,10 @@ func FromContext(ctx *ucli.Context) (*Options, error) { options.username = ctx.String(FlagUsername) } + if ctx.IsSet(FlagVerifySignature) { + options.verifySignature = ctx.Bool(FlagVerifySignature) + } + options.password = os.Getenv(cli.EnvKeyPassword) platform, err := cli.ParsePlatform(ctx.String(FlagPlatform)) diff --git a/internal/pkg/cli/puller/options_test.go b/internal/pkg/cli/puller/options_test.go index 293a79f759..a85931bd19 100644 --- a/internal/pkg/cli/puller/options_test.go +++ b/internal/pkg/cli/puller/options_test.go @@ -29,7 +29,7 @@ func TestFromContext(t *testing.T) { for _, tc := range []struct { name string prepare func(*flag.FlagSet) - assert func(error) + assert func(*Options, error) }{ { name: "success", @@ -38,8 +38,23 @@ func TestFromContext(t *testing.T) { require.Nil(t, set.Set(FlagUsername, "username")) require.Nil(t, set.Parse([]string{"echo"})) }, - assert: func(err error) { + assert: func(opts *Options, err error) { require.NoError(t, err) + require.True(t, opts.verifySignature) + }, + }, + { + name: "success with verify signature disabled", + prepare: func(set *flag.FlagSet) { + set.String(FlagUsername, "", "") + set.Bool(FlagVerifySignature, true, "") + require.Nil(t, set.Set(FlagUsername, "username")) + require.Nil(t, set.Set(FlagVerifySignature, "false")) + require.Nil(t, set.Parse([]string{"echo"})) + }, + assert: func(opts *Options, err error) { + require.NoError(t, err) + require.False(t, opts.verifySignature) }, }, { @@ -48,7 +63,7 @@ func TestFromContext(t *testing.T) { set.String(FlagOutputFile, "", "") require.Nil(t, set.Set(FlagOutputFile, "")) }, - assert: func(err error) { + assert: func(_ *Options, err error) { require.Error(t, err) }, }, @@ -59,7 +74,18 @@ func TestFromContext(t *testing.T) { require.Nil(t, set.Set(FlagOutputFile, "")) require.Nil(t, set.Parse([]string{"echo"})) }, - assert: func(err error) { + assert: func(_ *Options, err error) { + require.Error(t, err) + }, + }, + { + name: "failure parse platform", + prepare: func(set *flag.FlagSet) { + set.String(FlagPlatform, "", "") + require.Nil(t, set.Set(FlagPlatform, "os//var")) + require.Nil(t, set.Parse([]string{"echo"})) + }, + assert: func(_ *Options, err error) { require.Error(t, err) }, }, @@ -76,8 +102,8 @@ func TestFromContext(t *testing.T) { app := cli.NewApp() ctx := cli.NewContext(app, set, nil) - _, err := FromContext(ctx) - assert(err) + opts, err := FromContext(ctx) + assert(opts, err) }) } } diff --git a/internal/pkg/cli/puller/puller.go b/internal/pkg/cli/puller/puller.go index 4b41830082..5256309ae2 100644 --- a/internal/pkg/cli/puller/puller.go +++ b/internal/pkg/cli/puller/puller.go @@ -47,6 +47,7 @@ func (p *Puller) Run() error { p.options.username, p.options.password, p.options.platform, + p.options.verifySignature, ) if err != nil { return fmt.Errorf("pull profile: %w", err) diff --git a/internal/pkg/cli/puller/pullerfakes/fake_impl.go b/internal/pkg/cli/puller/pullerfakes/fake_impl.go index 07e707e48e..1565d10be3 100644 --- a/internal/pkg/cli/puller/pullerfakes/fake_impl.go +++ b/internal/pkg/cli/puller/pullerfakes/fake_impl.go @@ -26,13 +26,14 @@ import ( ) type FakeImpl struct { - PullStub func(string, string, string, *v1.Platform) (*artifact.PullResult, error) + PullStub func(string, string, string, *v1.Platform, bool) (*artifact.PullResult, error) pullMutex sync.RWMutex pullArgsForCall []struct { arg1 string arg2 string arg3 string arg4 *v1.Platform + arg5 bool } pullReturns struct { result1 *artifact.PullResult @@ -59,7 +60,7 @@ type FakeImpl struct { invocationsMutex sync.RWMutex } -func (fake *FakeImpl) Pull(arg1 string, arg2 string, arg3 string, arg4 *v1.Platform) (*artifact.PullResult, error) { +func (fake *FakeImpl) Pull(arg1 string, arg2 string, arg3 string, arg4 *v1.Platform, arg5 bool) (*artifact.PullResult, error) { fake.pullMutex.Lock() ret, specificReturn := fake.pullReturnsOnCall[len(fake.pullArgsForCall)] fake.pullArgsForCall = append(fake.pullArgsForCall, struct { @@ -67,13 +68,14 @@ func (fake *FakeImpl) Pull(arg1 string, arg2 string, arg3 string, arg4 *v1.Platf arg2 string arg3 string arg4 *v1.Platform - }{arg1, arg2, arg3, arg4}) + arg5 bool + }{arg1, arg2, arg3, arg4, arg5}) stub := fake.PullStub fakeReturns := fake.pullReturns - fake.recordInvocation("Pull", []interface{}{arg1, arg2, arg3, arg4}) + fake.recordInvocation("Pull", []interface{}{arg1, arg2, arg3, arg4, arg5}) fake.pullMutex.Unlock() if stub != nil { - return stub(arg1, arg2, arg3, arg4) + return stub(arg1, arg2, arg3, arg4, arg5) } if specificReturn { return ret.result1, ret.result2 @@ -87,17 +89,17 @@ func (fake *FakeImpl) PullCallCount() int { return len(fake.pullArgsForCall) } -func (fake *FakeImpl) PullCalls(stub func(string, string, string, *v1.Platform) (*artifact.PullResult, error)) { +func (fake *FakeImpl) PullCalls(stub func(string, string, string, *v1.Platform, bool) (*artifact.PullResult, error)) { fake.pullMutex.Lock() defer fake.pullMutex.Unlock() fake.PullStub = stub } -func (fake *FakeImpl) PullArgsForCall(i int) (string, string, string, *v1.Platform) { +func (fake *FakeImpl) PullArgsForCall(i int) (string, string, string, *v1.Platform, bool) { fake.pullMutex.RLock() defer fake.pullMutex.RUnlock() argsForCall := fake.pullArgsForCall[i] - return argsForCall.arg1, argsForCall.arg2, argsForCall.arg3, argsForCall.arg4 + return argsForCall.arg1, argsForCall.arg2, argsForCall.arg3, argsForCall.arg4, argsForCall.arg5 } func (fake *FakeImpl) PullReturns(result1 *artifact.PullResult, result2 error) { diff --git a/internal/pkg/daemon/seccompprofile/impl.go b/internal/pkg/daemon/seccompprofile/impl.go index 6a23882d51..3b13236b5c 100644 --- a/internal/pkg/daemon/seccompprofile/impl.go +++ b/internal/pkg/daemon/seccompprofile/impl.go @@ -35,7 +35,7 @@ type defaultImpl struct{} //go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 -generate -header ../../../../hack/boilerplate/boilerplate.generatego.txt //counterfeiter:generate . impl type impl interface { - Pull(context.Context, logr.Logger, string, string, string, *v1.Platform) (*artifact.PullResult, error) + Pull(context.Context, logr.Logger, string, string, string, *v1.Platform, bool) (*artifact.PullResult, error) PullResultType(*artifact.PullResult) artifact.PullResultType PullResultSeccompProfile(*artifact.PullResult) *seccompprofileapi.SeccompProfile ClientGetProfile( @@ -46,9 +46,9 @@ type impl interface { } func (*defaultImpl) Pull( - ctx context.Context, l logr.Logger, from, _, _ string, platform *v1.Platform, + ctx context.Context, l logr.Logger, from, username, password string, platform *v1.Platform, verifySignature bool, ) (*artifact.PullResult, error) { - return artifact.New(l).Pull(ctx, from, "", "", platform) + return artifact.New(l).Pull(ctx, from, username, password, platform, verifySignature) } func (*defaultImpl) PullResultType(res *artifact.PullResult) artifact.PullResultType { diff --git a/internal/pkg/daemon/seccompprofile/seccompprofile.go b/internal/pkg/daemon/seccompprofile/seccompprofile.go index bd7eeac8e4..f53e1f391d 100644 --- a/internal/pkg/daemon/seccompprofile/seccompprofile.go +++ b/internal/pkg/daemon/seccompprofile/seccompprofile.go @@ -367,7 +367,7 @@ func (r *Reconciler) resolveSyscallsForProfile( res, err := r.Pull(ctx, l, from, "", "", &v1.Platform{ Architecture: runtime.GOARCH, OS: runtime.GOOS, - }) + }, false /* TODO: make configurable */) if err != nil { l.Error(err, "cannot pull base profile "+baseProfileName) r.IncSeccompProfileError(r.metrics, reasonCannotPullProfile) diff --git a/internal/pkg/daemon/seccompprofile/seccompprofilefakes/fake_impl.go b/internal/pkg/daemon/seccompprofile/seccompprofilefakes/fake_impl.go index d417f2d4e7..517f38ebec 100644 --- a/internal/pkg/daemon/seccompprofile/seccompprofilefakes/fake_impl.go +++ b/internal/pkg/daemon/seccompprofile/seccompprofilefakes/fake_impl.go @@ -55,7 +55,7 @@ type FakeImpl struct { arg1 *metrics.Metrics arg2 string } - PullStub func(context.Context, logr.Logger, string, string, string, *v1.Platform) (*artifact.PullResult, error) + PullStub func(context.Context, logr.Logger, string, string, string, *v1.Platform, bool) (*artifact.PullResult, error) pullMutex sync.RWMutex pullArgsForCall []struct { arg1 context.Context @@ -64,6 +64,7 @@ type FakeImpl struct { arg4 string arg5 string arg6 *v1.Platform + arg7 bool } pullReturns struct { result1 *artifact.PullResult @@ -208,7 +209,7 @@ func (fake *FakeImpl) IncSeccompProfileErrorArgsForCall(i int) (*metrics.Metrics return argsForCall.arg1, argsForCall.arg2 } -func (fake *FakeImpl) Pull(arg1 context.Context, arg2 logr.Logger, arg3 string, arg4 string, arg5 string, arg6 *v1.Platform) (*artifact.PullResult, error) { +func (fake *FakeImpl) Pull(arg1 context.Context, arg2 logr.Logger, arg3 string, arg4 string, arg5 string, arg6 *v1.Platform, arg7 bool) (*artifact.PullResult, error) { fake.pullMutex.Lock() ret, specificReturn := fake.pullReturnsOnCall[len(fake.pullArgsForCall)] fake.pullArgsForCall = append(fake.pullArgsForCall, struct { @@ -218,13 +219,14 @@ func (fake *FakeImpl) Pull(arg1 context.Context, arg2 logr.Logger, arg3 string, arg4 string arg5 string arg6 *v1.Platform - }{arg1, arg2, arg3, arg4, arg5, arg6}) + arg7 bool + }{arg1, arg2, arg3, arg4, arg5, arg6, arg7}) stub := fake.PullStub fakeReturns := fake.pullReturns - fake.recordInvocation("Pull", []interface{}{arg1, arg2, arg3, arg4, arg5, arg6}) + fake.recordInvocation("Pull", []interface{}{arg1, arg2, arg3, arg4, arg5, arg6, arg7}) fake.pullMutex.Unlock() if stub != nil { - return stub(arg1, arg2, arg3, arg4, arg5, arg6) + return stub(arg1, arg2, arg3, arg4, arg5, arg6, arg7) } if specificReturn { return ret.result1, ret.result2 @@ -238,17 +240,17 @@ func (fake *FakeImpl) PullCallCount() int { return len(fake.pullArgsForCall) } -func (fake *FakeImpl) PullCalls(stub func(context.Context, logr.Logger, string, string, string, *v1.Platform) (*artifact.PullResult, error)) { +func (fake *FakeImpl) PullCalls(stub func(context.Context, logr.Logger, string, string, string, *v1.Platform, bool) (*artifact.PullResult, error)) { fake.pullMutex.Lock() defer fake.pullMutex.Unlock() fake.PullStub = stub } -func (fake *FakeImpl) PullArgsForCall(i int) (context.Context, logr.Logger, string, string, string, *v1.Platform) { +func (fake *FakeImpl) PullArgsForCall(i int) (context.Context, logr.Logger, string, string, string, *v1.Platform, bool) { fake.pullMutex.RLock() defer fake.pullMutex.RUnlock() argsForCall := fake.pullArgsForCall[i] - return argsForCall.arg1, argsForCall.arg2, argsForCall.arg3, argsForCall.arg4, argsForCall.arg5, argsForCall.arg6 + return argsForCall.arg1, argsForCall.arg2, argsForCall.arg3, argsForCall.arg4, argsForCall.arg5, argsForCall.arg6, argsForCall.arg7 } func (fake *FakeImpl) PullReturns(result1 *artifact.PullResult, result2 error) { diff --git a/internal/pkg/manager/spod/bindata/spod.go b/internal/pkg/manager/spod/bindata/spod.go index 917e251bde..4f2be0f238 100644 --- a/internal/pkg/manager/spod/bindata/spod.go +++ b/internal/pkg/manager/spod/bindata/spod.go @@ -369,7 +369,7 @@ semodule -i /opt/spo-profiles/selinuxrecording.cil }, }, { - Name: "OPERATOR_NAMESPACE", + Name: config.OperatorNamespaceEnvKey, ValueFrom: &corev1.EnvVarSource{ FieldRef: &corev1.ObjectFieldSelector{ FieldPath: "metadata.namespace", diff --git a/internal/pkg/manager/spod/bindata/webhook.go b/internal/pkg/manager/spod/bindata/webhook.go index 2c779f10e3..1e2c4a2224 100644 --- a/internal/pkg/manager/spod/bindata/webhook.go +++ b/internal/pkg/manager/spod/bindata/webhook.go @@ -341,7 +341,7 @@ var webhookDeployment = &appsv1.Deployment{ }, Env: []corev1.EnvVar{ { - Name: "OPERATOR_NAMESPACE", + Name: config.OperatorNamespaceEnvKey, ValueFrom: &corev1.EnvVarSource{ FieldRef: &corev1.ObjectFieldSelector{ FieldPath: "metadata.namespace", diff --git a/test/tc_base_profiles_oci_test.go b/test/tc_base_profiles_oci_test.go index c93e5b496c..0d9b5ba217 100644 --- a/test/tc_base_profiles_oci_test.go +++ b/test/tc_base_profiles_oci_test.go @@ -84,6 +84,8 @@ spec: e.logf("Waiting for profile to be reconciled") e.waitFor("condition=ready", "sp", "hello") + e.kubectlOperatorNS("logs", "-l", "name=spod") + e.logf("Creating hello-world pod") helloPodFile, err := os.CreateTemp("", "hello-pod*.yaml") e.Nil(err)