From abef8d6a5ccc765ac0f3599e00e04a9bf36b458f Mon Sep 17 00:00:00 2001 From: David Gannon <19214156+dgannon991@users.noreply.github.com> Date: Wed, 29 Jan 2025 21:58:38 +0000 Subject: [PATCH] Made context a mandatory parameter instead Signed-off-by: David Gannon <19214156+dgannon991@users.noreply.github.com> --- Makefile | 2 +- action/action.go | 15 ++++++++---- action/action_test.go | 23 ++++++++++--------- action/example_install_test.go | 3 ++- action/example_invoke_test.go | 6 ++--- action/example_running_status_test.go | 3 ++- action/example_upgrade_test.go | 3 ++- driver/command/command.go | 3 ++- driver/command/command_nix_test.go | 9 ++++---- driver/debug/debug.go | 3 ++- driver/debug/debug_test.go | 3 ++- driver/docker/docker.go | 7 +++--- driver/docker/docker_integration_test.go | 10 ++++---- driver/driver.go | 3 ++- driver/kubernetes/kubernetes.go | 3 +-- .../kubernetes/kubernetes_integration_test.go | 2 +- driver/kubernetes/kubernetes_test.go | 6 ++--- golangci.yml | 3 ++- .../testdata/artifacts/layout/index.json | 16 +++++++++++++ 19 files changed, 75 insertions(+), 48 deletions(-) diff --git a/Makefile b/Makefile index 58734ec9..3fddadcc 100644 --- a/Makefile +++ b/Makefile @@ -34,7 +34,7 @@ delete-test-cluster: GOPATH := $(shell go env GOPATH) HAS_GOLANGCI := $(shell $(CHECK) golangci-lint) -GOLANGCI_VERSION := v1.51.2 +GOLANGCI_VERSION := v1.63.4 HAS_KIND := $(shell $(CHECK) kind) HAS_KUBECTL := $(shell $(CHECK) kubectl) HAS_GOCOV_XML := $(shell $(CHECK) gocov-xml;) diff --git a/action/action.go b/action/action.go index 0156a2ad..ed77c870 100644 --- a/action/action.go +++ b/action/action.go @@ -1,6 +1,7 @@ package action import ( + "context" "crypto/sha256" "encoding/hex" "encoding/json" @@ -53,7 +54,7 @@ func New(d driver.Driver) Action { // caller is responsible for persisting the claim records and outputs using the // SaveOperationResult function. An error is only returned when the operation could not // be executed, otherwise any error is returned in the OperationResult. -func (a Action) Run(c claim.Claim, creds valuesource.Set, opCfgs ...OperationConfigFunc) (driver.OperationResult, claim.Result, error) { +func (a Action) Run(ctx context.Context, c claim.Claim, creds valuesource.Set, opCfgs ...OperationConfigFunc) (driver.OperationResult, claim.Result, error) { if a.Driver == nil { return driver.OperationResult{}, claim.Result{}, errors.New("the action driver is not set") } @@ -84,7 +85,7 @@ func (a Action) Run(c claim.Claim, creds valuesource.Set, opCfgs ...OperationCon } var opErr *multierror.Error - opResult, err := a.Driver.Run(op) + opResult, err := a.Driver.Run(ctx, op) if err != nil { opErr = multierror.Append(opErr, err) } @@ -279,7 +280,10 @@ func setOutputsOnClaimResult(c claim.Claim, result *claim.Result, opResult drive for outputName, outputValue := range opResult.Outputs { outputDef, isDefined := c.Bundle.Outputs[outputName] - result.OutputMetadata.SetGeneratedByBundle(outputName, isDefined) + err := result.OutputMetadata.SetGeneratedByBundle(outputName, isDefined) + if err != nil { + outputErrors = append(outputErrors, err) + } if isDefined { err := validateOutputType(c.Bundle, outputName, outputDef, outputValue) if err != nil { @@ -288,7 +292,10 @@ func setOutputsOnClaimResult(c claim.Claim, result *claim.Result, opResult drive } if outputValue != "" { - result.OutputMetadata.SetContentDigest(outputName, buildOutputContentDigest(outputValue)) + err := result.OutputMetadata.SetContentDigest(outputName, buildOutputContentDigest(outputValue)) + if err != nil { + outputErrors = append(outputErrors, err) + } } } diff --git a/action/action_test.go b/action/action_test.go index 2f6c68c5..2184ef84 100644 --- a/action/action_test.go +++ b/action/action_test.go @@ -1,6 +1,7 @@ package action import ( + "context" "encoding/json" "errors" "fmt" @@ -31,7 +32,7 @@ type mockDriver struct { func (d *mockDriver) Handles(imageType string) bool { return d.shouldHandle } -func (d *mockDriver) Run(op *driver.Operation) (driver.OperationResult, error) { +func (d *mockDriver) Run(ctx context.Context, op *driver.Operation) (driver.OperationResult, error) { d.Operation = op fmt.Fprintln(op.Out, "mocked running the bundle") return d.Result, d.Error @@ -720,7 +721,7 @@ func TestAction_RunAction(t *testing.T) { inst := New(d) inst.SaveLogs = true - opResult, claimResult, err := inst.Run(c, mockSet, out) + opResult, claimResult, err := inst.Run(context.Background(), c, mockSet, out) require.NoError(t, err) require.NoError(t, opResult.Error) assert.Equal(t, claim.ActionInstall, c.Action) @@ -748,7 +749,7 @@ func TestAction_RunAction(t *testing.T) { inst := New(d) inst.SaveLogs = false - opResult, _, err := inst.Run(c, mockSet, out) + opResult, _, err := inst.Run(context.Background(), c, mockSet, out) require.NoError(t, err) require.NoError(t, opResult.Error) @@ -772,7 +773,7 @@ func TestAction_RunAction(t *testing.T) { op.Files["/tmp/another/path"] = "ANOTHER FILE" return nil } - _, _, err := inst.Run(c, mockSet, out, addFile) + _, _, err := inst.Run(context.Background(), c, mockSet, out, addFile) require.NoError(t, err) assert.Contains(t, d.Operation.Files, "/tmp/another/path") }) @@ -792,7 +793,7 @@ func TestAction_RunAction(t *testing.T) { sabotage := func(op *driver.Operation) error { return errors.New("oops") } - _, _, err := inst.Run(c, mockSet, out, sabotage) + _, _, err := inst.Run(context.Background(), c, mockSet, out, sabotage) require.EqualError(t, err, "oops") }) @@ -805,7 +806,7 @@ func TestAction_RunAction(t *testing.T) { Error: nil, } inst := New(d) - _, claimResult, err := inst.Run(c, mockSet, out) + _, claimResult, err := inst.Run(context.Background(), c, mockSet, out) require.NoError(t, err) assert.Equal(t, claim.ActionInstall, c.Action) assert.Equal(t, claim.StatusSucceeded, claimResult.Status) @@ -848,7 +849,7 @@ func TestAction_RunAction(t *testing.T) { Error: nil, } inst := New(d) - opResult, _, err := inst.Run(c, mockSet, out) + opResult, _, err := inst.Run(context.Background(), c, mockSet, out) require.NoError(t, err) assert.Contains(t, opResult.Outputs, "hasDefault1", "the output always applies so an output value should have been set") @@ -873,7 +874,7 @@ func TestAction_RunAction(t *testing.T) { Error: nil, } inst := New(d) - opResult, _, err := inst.Run(c, mockSet, out) + opResult, _, err := inst.Run(context.Background(), c, mockSet, out) require.NoError(t, err) require.Contains(t, opResult.Error.Error(), "required output noDefault is missing and has no default") }) @@ -885,7 +886,7 @@ func TestAction_RunAction(t *testing.T) { Error: errors.New("I always fail"), } inst := New(d) - _, _, err := inst.Run(c, mockSet, out) + _, _, err := inst.Run(context.Background(), c, mockSet, out) require.Error(t, err) }) @@ -901,7 +902,7 @@ func TestAction_RunAction(t *testing.T) { Error: errors.New("I always fail"), } inst := New(d) - opResult, claimResult, err := inst.Run(c, mockSet, out) + opResult, claimResult, err := inst.Run(context.Background(), c, mockSet, out) require.NoError(t, err) require.Contains(t, opResult.Error.Error(), "I always fail") assert.Equal(t, claim.ActionInstall, c.Action) @@ -922,7 +923,7 @@ func TestAction_RunAction(t *testing.T) { Error: errors.New("I always fail"), } inst := New(d) - opResult, claimResult, err := inst.Run(c, mockSet, out) + opResult, claimResult, err := inst.Run(context.Background(), c, mockSet, out) require.Error(t, err, "Unknown action should fail") require.NoError(t, opResult.Error) assert.Empty(t, claimResult) diff --git a/action/example_install_test.go b/action/example_install_test.go index ae6b42c9..1c8beca2 100644 --- a/action/example_install_test.go +++ b/action/example_install_test.go @@ -1,6 +1,7 @@ package action_test import ( + "context" "fmt" "time" @@ -61,7 +62,7 @@ func Example_install() { // Pass an empty set of credentials var creds valuesource.Set - opResult, claimResult, err := a.Run(c, creds) + opResult, claimResult, err := a.Run(context.Background(), c, creds) if err != nil { // Something terrible has occurred and we could not even run the bundle panic(err) diff --git a/action/example_invoke_test.go b/action/example_invoke_test.go index 08fb395d..bef75eef 100644 --- a/action/example_invoke_test.go +++ b/action/example_invoke_test.go @@ -1,6 +1,7 @@ package action_test import ( + "context" "fmt" "time" @@ -23,9 +24,6 @@ func Example_invoke() { // Load the previous claim for the installation existingClaim := createInstallClaim() - if err != nil { - panic(err) - } // Create a claim for running the custom logs action based on the previous claim c, err := existingClaim.NewClaim("logs", existingClaim.Bundle, parameters) @@ -51,7 +49,7 @@ func Example_invoke() { // Pass an empty set of credentials var creds valuesource.Set - opResult, claimResult, err := a.Run(c, creds) + opResult, claimResult, err := a.Run(context.Background(), c, creds) if err != nil { // Something terrible has occurred and we could not even run the bundle panic(err) diff --git a/action/example_running_status_test.go b/action/example_running_status_test.go index d3a31414..f92684b2 100644 --- a/action/example_running_status_test.go +++ b/action/example_running_status_test.go @@ -1,6 +1,7 @@ package action_test import ( + "context" "fmt" "time" @@ -56,7 +57,7 @@ func Example_runningStatus() { // Save the upgrade claim in the Running Status saveResult(c, claim.StatusRunning) - opResult, claimResult, err := a.Run(c, creds) + opResult, claimResult, err := a.Run(context.Background(), c, creds) if err != nil { // If the bundle isn't run due to an error preparing, // record a failure so we aren't left stuck in running diff --git a/action/example_upgrade_test.go b/action/example_upgrade_test.go index d86a2869..caca7bff 100644 --- a/action/example_upgrade_test.go +++ b/action/example_upgrade_test.go @@ -1,6 +1,7 @@ package action_test import ( + "context" "fmt" "time" @@ -44,7 +45,7 @@ func Example_upgrade() { // Pass an empty set of credentials var creds valuesource.Set - opResult, claimResult, err := a.Run(c, creds) + opResult, claimResult, err := a.Run(context.Background(), c, creds) if err != nil { // Something terrible has occurred and we could not even run the bundle panic(err) diff --git a/driver/command/command.go b/driver/command/command.go index 0648e6ca..691f842e 100644 --- a/driver/command/command.go +++ b/driver/command/command.go @@ -2,6 +2,7 @@ package command import ( "bytes" + "context" "encoding/json" "fmt" "io" @@ -26,7 +27,7 @@ type Driver struct { } // Run executes the command -func (d *Driver) Run(op *driver.Operation) (driver.OperationResult, error) { +func (d *Driver) Run(ctx context.Context, op *driver.Operation) (driver.OperationResult, error) { return d.exec(op) } diff --git a/driver/command/command_nix_test.go b/driver/command/command_nix_test.go index 3f8209dc..bcef6f1e 100644 --- a/driver/command/command_nix_test.go +++ b/driver/command/command_nix_test.go @@ -4,6 +4,7 @@ package command import ( + "context" "os" "testing" @@ -64,7 +65,7 @@ func TestCommandDriverOutputs(t *testing.T) { t.Fatalf("Expected driver %s to exist Driver Name %s ", name, cmddriver.Name) } op := buildOp() - opResult, err := cmddriver.Run(op) + opResult, err := cmddriver.Run(context.Background(), op) if err != nil { t.Fatalf("Driver Run failed %v", err) } @@ -89,7 +90,7 @@ func TestCommandDriverOutputs(t *testing.T) { t.Fatalf("Expected driver %s to exist Driver Name %s ", name, cmddriver.Name) } op := buildOp() - opResult, err := cmddriver.Run(op) + opResult, err := cmddriver.Run(context.Background(), op) if err != nil { t.Fatalf("Driver Run failed %v", err) } @@ -114,7 +115,7 @@ func TestCommandDriverOutputs(t *testing.T) { t.Fatalf("Expected driver %s to exist Driver Name %s ", name, cmddriver.Name) } op := buildOp() - _, err := cmddriver.Run(op) + _, err := cmddriver.Run(context.Background(), op) assert.NoError(t, err) } CreateAndRunTestCommandDriver(t, name, false, content, testfunc) @@ -133,7 +134,7 @@ func TestCommandDriverOutputs(t *testing.T) { } op := buildOp() op.Bundle.Definitions["output2"].Default = "DEFAULT OUTPUT 2" - opResult, err := cmddriver.Run(op) + opResult, err := cmddriver.Run(context.Background(), op) if err != nil { t.Fatalf("Driver Run failed %v", err) } diff --git a/driver/debug/debug.go b/driver/debug/debug.go index 11cfa601..72a3be5f 100644 --- a/driver/debug/debug.go +++ b/driver/debug/debug.go @@ -1,6 +1,7 @@ package debug import ( + "context" "encoding/json" "fmt" @@ -15,7 +16,7 @@ type Driver struct { } // Run executes the operation on the Debug driver -func (d *Driver) Run(op *driver.Operation) (driver.OperationResult, error) { +func (d *Driver) Run(ctx context.Context, op *driver.Operation) (driver.OperationResult, error) { data, err := json.MarshalIndent(op, "", " ") if err != nil { return driver.OperationResult{}, err diff --git a/driver/debug/debug_test.go b/driver/debug/debug_test.go index c683d05f..9178efb6 100644 --- a/driver/debug/debug_test.go +++ b/driver/debug/debug_test.go @@ -1,6 +1,7 @@ package debug import ( + "context" "io/ioutil" "testing" @@ -36,6 +37,6 @@ func TestDebugDriver_Run(t *testing.T) { Out: ioutil.Discard, } - _, err := d.Run(op) + _, err := d.Run(context.Background(), op) is.NoError(err) } diff --git a/driver/docker/docker.go b/driver/docker/docker.go index 33edcd2f..c8a9b349 100644 --- a/driver/docker/docker.go +++ b/driver/docker/docker.go @@ -49,8 +49,8 @@ type Driver struct { } // Run executes the Docker driver -func (d *Driver) Run(op *driver.Operation) (driver.OperationResult, error) { - return d.exec(op) +func (d *Driver) Run(ctx context.Context, op *driver.Operation) (driver.OperationResult, error) { + return d.exec(ctx, op) } // Handles indicates that the Docker driver supports "docker" and "oci" @@ -181,8 +181,7 @@ func (d *Driver) initializeDockerCli() (command.Cli, error) { return cli, nil } -func (d *Driver) exec(op *driver.Operation) (driver.OperationResult, error) { - ctx := context.Background() +func (d *Driver) exec(ctx context.Context, op *driver.Operation) (driver.OperationResult, error) { cli, err := d.initializeDockerCli() if err != nil { diff --git a/driver/docker/docker_integration_test.go b/driver/docker/docker_integration_test.go index c7f31db9..d2c50c6b 100644 --- a/driver/docker/docker_integration_test.go +++ b/driver/docker/docker_integration_test.go @@ -1,10 +1,8 @@ -//go:build integration -// +build integration - package docker import ( "bytes" + "context" "fmt" "io" "os" @@ -92,7 +90,7 @@ func runDriverTest(t *testing.T, image bundle.InvocationImage, skipValidations b } docker := &Driver{} - opResult, err := docker.Run(op) + opResult, err := docker.Run(context.Background(), op) if skipValidations { return @@ -155,7 +153,7 @@ func TestDriver_Run_CaptureOutput(t *testing.T) { } docker := &Driver{} - _, err := docker.Run(op) + _, err := docker.Run(context.Background(), op) assert.NoError(t, err) assert.Equal(t, "installing bundle...\n", stdout.String()) @@ -190,7 +188,7 @@ func TestDriver_ValidateImageDigestFail(t *testing.T) { docker := &Driver{} - _, err := docker.Run(op) + _, err := docker.Run(context.Background(), op) require.Error(t, err, "expected an error") // Not asserting actual image digests to support arbitrary integration test images assert.Contains(t, err.Error(), diff --git a/driver/driver.go b/driver/driver.go index 48e38ff9..3521aad0 100644 --- a/driver/driver.go +++ b/driver/driver.go @@ -1,6 +1,7 @@ package driver import ( + "context" "fmt" "io" @@ -88,7 +89,7 @@ func (r *OperationResult) SetDefaultOutputValues(op Operation) error { // Driver is capable of running a invocation image type Driver interface { // Run executes the operation inside of the invocation image - Run(*Operation) (OperationResult, error) + Run(context.Context, *Operation) (OperationResult, error) // Handles receives an ImageType* and answers whether this driver supports that type Handles(string) bool } diff --git a/driver/kubernetes/kubernetes.go b/driver/kubernetes/kubernetes.go index 9db4175b..3c686f7e 100644 --- a/driver/kubernetes/kubernetes.go +++ b/driver/kubernetes/kubernetes.go @@ -298,13 +298,12 @@ func (k *Driver) setClient(conf *rest.Config) error { } // Run executes the operation inside of the invocation image. -func (k *Driver) Run(op *driver.Operation) (driver.OperationResult, error) { +func (k *Driver) Run(ctx context.Context, op *driver.Operation) (driver.OperationResult, error) { err := k.initClient() if err != nil { return driver.OperationResult{}, err } - ctx := context.Background() const sharedVolumeName = "cnab-driver-share" err = k.initJobVolumes() if err != nil { diff --git a/driver/kubernetes/kubernetes_integration_test.go b/driver/kubernetes/kubernetes_integration_test.go index aeddb248..e72a1264 100644 --- a/driver/kubernetes/kubernetes_integration_test.go +++ b/driver/kubernetes/kubernetes_integration_test.go @@ -166,7 +166,7 @@ func TestDriver_Run_Integration(t *testing.T) { defer deletePod() } - _, err = k.Run(tc.op) + _, err = k.Run(context.Background(), tc.op) if tc.err != nil { assert.EqualError(t, err, tc.err.Error()) diff --git a/driver/kubernetes/kubernetes_test.go b/driver/kubernetes/kubernetes_test.go index 4b8ac84d..4adc0157 100644 --- a/driver/kubernetes/kubernetes_test.go +++ b/driver/kubernetes/kubernetes_test.go @@ -45,7 +45,7 @@ func TestDriver_Run(t *testing.T) { }, } - _, err = k.Run(&op) + _, err = k.Run(context.Background(), &op) assert.NoError(t, err) jobList, _ := k.jobs.List(ctx, metav1.ListOptions{}) @@ -103,7 +103,7 @@ func TestDriver_RunWithSharedFiles(t *testing.T) { }, } - opResult, err := k.Run(&op) + opResult, err := k.Run(context.Background(), &op) require.NoError(t, err) jobList, _ := k.jobs.List(ctx, metav1.ListOptions{}) @@ -271,7 +271,7 @@ func TestDriver_ConfigureJob(t *testing.T) { Image: bundle.InvocationImage{BaseImage: bundle.BaseImage{Image: "foo/bar"}}, } - _, err = k.Run(&op) + _, err = k.Run(context.Background(), &op) assert.NoError(t, err) jobList, _ := k.jobs.List(ctx, metav1.ListOptions{}) diff --git a/golangci.yml b/golangci.yml index e9968cba..057d7349 100644 --- a/golangci.yml +++ b/golangci.yml @@ -1,5 +1,6 @@ run: - deadline: 2m + deadline: 5m + timeout: 10m linters: disable-all: true diff --git a/imagestore/ocilayout/testdata/artifacts/layout/index.json b/imagestore/ocilayout/testdata/artifacts/layout/index.json index ceff5328..3ac0ee81 100755 --- a/imagestore/ocilayout/testdata/artifacts/layout/index.json +++ b/imagestore/ocilayout/testdata/artifacts/layout/index.json @@ -33,6 +33,22 @@ "org.opencontainers.image.ref.name": "docker.io/library/hello-world@sha256:f54a58bc1aac5ea1a25d796ae155dc228b3f0e11d046ae276b39c4bf2f13d8c4" } }, + { + "mediaType": "application/vnd.docker.distribution.manifest.v2+json", + "size": 525, + "digest": "sha256:f54a58bc1aac5ea1a25d796ae155dc228b3f0e11d046ae276b39c4bf2f13d8c4", + "annotations": { + "org.opencontainers.image.ref.name": "docker.io/library/hello-world@sha256:f54a58bc1aac5ea1a25d796ae155dc228b3f0e11d046ae276b39c4bf2f13d8c4" + } + }, + { + "mediaType": "application/vnd.docker.distribution.manifest.v2+json", + "size": 525, + "digest": "sha256:f54a58bc1aac5ea1a25d796ae155dc228b3f0e11d046ae276b39c4bf2f13d8c4", + "annotations": { + "org.opencontainers.image.ref.name": "docker.io/library/hello-world@sha256:f54a58bc1aac5ea1a25d796ae155dc228b3f0e11d046ae276b39c4bf2f13d8c4" + } + }, { "mediaType": "application/vnd.docker.distribution.manifest.v2+json", "size": 525,