From 3d66a3f7e75600686c5391cf959ccb7efc6cdd90 Mon Sep 17 00:00:00 2001 From: Carolyn Van Slyck Date: Wed, 20 Jul 2022 08:18:52 -0500 Subject: [PATCH] Context plumbing (#2242) * Add context to PullBundle Signed-off-by: Carolyn Van Slyck * Add context to Copy Signed-off-by: Carolyn Van Slyck * Use context in build Signed-off-by: Carolyn Van Slyck * Use context when working with shared cnab options Signed-off-by: Carolyn Van Slyck * Remove unhelpful debug statement Signed-off-by: Carolyn Van Slyck * Use context in version command Signed-off-by: Carolyn Van Slyck * Add context to porter storage permissions command Signed-off-by: Carolyn Van Slyck * Use context when printing debug info for a bundle Signed-off-by: Carolyn Van Slyck * Use context in IsBundleUpToDate Signed-off-by: Carolyn Van Slyck * Use context in schema command Signed-off-by: Carolyn Van Slyck * Add context to mixins commands Signed-off-by: Carolyn Van Slyck * Add context to plugins uninstall command Signed-off-by: Carolyn Van Slyck * Add context to the exec mixin commands (but not the mixins runtime) Signed-off-by: Carolyn Van Slyck * Use context in lint command Signed-off-by: Carolyn Van Slyck * Use context in remaining credentials commands Signed-off-by: Carolyn Van Slyck * Add context to pkgmgmt commands Signed-off-by: Carolyn Van Slyck * use context in random places Signed-off-by: Carolyn Van Slyck * Rest of the fixes Signed-off-by: Carolyn Van Slyck * Use context in dependencies Signed-off-by: Carolyn Van Slyck * Review feedback Signed-off-by: Carolyn Van Slyck * Do not print applied parameter/credential set Signed-off-by: Carolyn Van Slyck --- cmd/exec/install.go | 2 +- cmd/exec/invoke.go | 2 +- cmd/exec/lint.go | 2 +- cmd/exec/uninstall.go | 2 +- cmd/exec/upgrade.go | 2 +- cmd/porter/copy.go | 2 +- cmd/porter/credentials.go | 2 +- cmd/porter/mixins.go | 4 +- cmd/porter/plugins.go | 2 +- cmd/porter/storage.go | 2 +- pkg/cnab/cnab-to-oci/helpers.go | 2 +- pkg/cnab/cnab-to-oci/provider.go | 2 +- pkg/cnab/cnab-to-oci/registry.go | 16 +++- pkg/cnab/provider/action.go | 23 +++--- pkg/config/config.go | 11 ++- pkg/exec/builder/action.go | 19 +++-- pkg/exec/exec_test.go | 26 ++++--- pkg/exec/execute.go | 12 +-- pkg/exec/execute_test.go | 4 +- pkg/exec/lint.go | 9 ++- pkg/exec/lint_test.go | 4 +- pkg/linter/linter.go | 11 +-- pkg/mixin/pkgmgmt.go | 6 +- pkg/mixin/pkgmgmt_test.go | 1 - pkg/mixin/query/query.go | 10 ++- pkg/pkgmgmt/client/delete.go | 23 +++--- pkg/pkgmgmt/client/delete_test.go | 4 +- pkg/pkgmgmt/client/filesystem.go | 9 ++- pkg/pkgmgmt/client/helpers.go | 2 +- pkg/pkgmgmt/client/install.go | 2 +- pkg/pkgmgmt/client/runner.go | 28 +++---- pkg/pkgmgmt/feed/generate.go | 13 +++- pkg/pkgmgmt/feed/generate_test.go | 27 ++++--- pkg/pkgmgmt/feed/load.go | 37 ++++----- pkg/pkgmgmt/pkgmgmt.go | 2 +- pkg/plugins/pluggable/connection.go | 2 +- pkg/plugins/runner.go | 25 +++--- pkg/porter/build.go | 21 +++--- pkg/porter/copy.go | 20 +++-- pkg/porter/credentials.go | 96 +++++++++++++++--------- pkg/porter/credentials_test.go | 3 +- pkg/porter/dependencies.go | 54 ++++++++----- pkg/porter/examples/pull_example_test.go | 4 +- pkg/porter/lifecycle.go | 6 +- pkg/porter/lifecycle_test.go | 16 ++-- pkg/porter/mixins.go | 8 +- pkg/porter/mixins_test.go | 3 +- pkg/porter/parameters.go | 67 +++++++---------- pkg/porter/plugins.go | 4 +- pkg/porter/plugins_test.go | 3 +- pkg/porter/porter.go | 5 -- pkg/porter/porter_integration_test.go | 9 +-- pkg/porter/publish.go | 19 ++--- pkg/porter/pull.go | 5 +- pkg/porter/reconcile.go | 14 ++-- pkg/porter/resolver.go | 10 ++- pkg/porter/resolver_test.go | 10 ++- pkg/porter/schema.go | 59 ++++++++------- pkg/porter/stamp.go | 17 ++--- pkg/porter/storage.go | 20 +++-- pkg/porter/version.go | 13 ++-- tests/integration/pull_test.go | 4 +- tests/integration/uninstall_test.go | 3 - 63 files changed, 469 insertions(+), 376 deletions(-) diff --git a/cmd/exec/install.go b/cmd/exec/install.go index bd5d522c7..894e9d8bc 100644 --- a/cmd/exec/install.go +++ b/cmd/exec/install.go @@ -12,7 +12,7 @@ func buildInstallCommand(m *exec.Mixin) *cobra.Command { Use: "install", Short: "Execute the install functionality of this mixin", RunE: func(cmd *cobra.Command, args []string) error { - return m.Execute(opts) + return m.Execute(cmd.Context(), opts) }, } flags := cmd.Flags() diff --git a/cmd/exec/invoke.go b/cmd/exec/invoke.go index 1e9cacd88..05673d1e7 100644 --- a/cmd/exec/invoke.go +++ b/cmd/exec/invoke.go @@ -12,7 +12,7 @@ func buildInvokeCommand(m *exec.Mixin) *cobra.Command { Use: "invoke", Short: "Execute the invoke functionality of this mixin", RunE: func(cmd *cobra.Command, args []string) error { - return m.Execute(opts) + return m.Execute(cmd.Context(), opts) }, } flags := cmd.Flags() diff --git a/cmd/exec/lint.go b/cmd/exec/lint.go index c1d26dbd6..bbd0cf4a2 100644 --- a/cmd/exec/lint.go +++ b/cmd/exec/lint.go @@ -10,7 +10,7 @@ func buildLintCommand(m *exec.Mixin) *cobra.Command { Use: "lint", Short: "Check sections of the bundle associated with this mixin for problems and adherence to best practices", RunE: func(cmd *cobra.Command, args []string) error { - return m.PrintLintResults() + return m.PrintLintResults(cmd.Context()) }, } return cmd diff --git a/cmd/exec/uninstall.go b/cmd/exec/uninstall.go index 58176132c..96ffb9db2 100644 --- a/cmd/exec/uninstall.go +++ b/cmd/exec/uninstall.go @@ -12,7 +12,7 @@ func buildUninstallCommand(m *exec.Mixin) *cobra.Command { Use: "uninstall", Short: "Execute the uninstall functionality of this mixin", RunE: func(cmd *cobra.Command, args []string) error { - return m.Execute(opts) + return m.Execute(cmd.Context(), opts) }, } flags := cmd.Flags() diff --git a/cmd/exec/upgrade.go b/cmd/exec/upgrade.go index e7035c521..5eca53a3d 100644 --- a/cmd/exec/upgrade.go +++ b/cmd/exec/upgrade.go @@ -12,7 +12,7 @@ func buildUpgradeCommand(m *exec.Mixin) *cobra.Command { Use: "upgrade", Short: "Execute the upgrade functionality of this mixin", RunE: func(cmd *cobra.Command, args []string) error { - return m.Execute(opts) + return m.Execute(cmd.Context(), opts) }, } flags := cmd.Flags() diff --git a/cmd/porter/copy.go b/cmd/porter/copy.go index 9bca566bf..469725987 100644 --- a/cmd/porter/copy.go +++ b/cmd/porter/copy.go @@ -25,7 +25,7 @@ If the source bundle is a digest reference, destination must be a tagged referen return opts.Validate() }, RunE: func(cmd *cobra.Command, args []string) error { - return p.CopyBundle(opts) + return p.CopyBundle(cmd.Context(), opts) }, } f := cmd.Flags() diff --git a/cmd/porter/credentials.go b/cmd/porter/credentials.go index 30d878411..797914560 100644 --- a/cmd/porter/credentials.go +++ b/cmd/porter/credentials.go @@ -236,7 +236,7 @@ func buildCredentialsCreateCommand(p *porter.Porter) *cobra.Command { return opts.Validate(args) }, RunE: func(cmd *cobra.Command, argrs []string) error { - return p.CreateCredential(opts) + return p.CreateCredential(cmd.Context(), opts) }, } diff --git a/cmd/porter/mixins.go b/cmd/porter/mixins.go index 67e098bf4..0882b7bd0 100644 --- a/cmd/porter/mixins.go +++ b/cmd/porter/mixins.go @@ -120,7 +120,7 @@ func BuildMixinUninstallCommand(p *porter.Porter) *cobra.Command { return opts.Validate(args) }, RunE: func(cmd *cobra.Command, args []string) error { - return p.UninstallMixin(opts) + return p.UninstallMixin(cmd.Context(), opts) }, } @@ -172,7 +172,7 @@ See https://getporter.org/mixin-dev-guide/distribution more details. return opts.Validate(p.Context) }, RunE: func(cmd *cobra.Command, args []string) error { - return p.GenerateMixinFeed(opts) + return p.GenerateMixinFeed(cmd.Context(), opts) }, } diff --git a/cmd/porter/plugins.go b/cmd/porter/plugins.go index 75995f5a6..575299f05 100644 --- a/cmd/porter/plugins.go +++ b/cmd/porter/plugins.go @@ -142,7 +142,7 @@ func BuildPluginUninstallCommand(p *porter.Porter) *cobra.Command { return opts.Validate(args) }, RunE: func(cmd *cobra.Command, args []string) error { - return p.UninstallPlugin(opts) + return p.UninstallPlugin(cmd.Context(), opts) }, } diff --git a/cmd/porter/storage.go b/cmd/porter/storage.go index c4dbe51e4..7ab496f09 100644 --- a/cmd/porter/storage.go +++ b/cmd/porter/storage.go @@ -64,7 +64,7 @@ func buildStorageFixPermissionsCommand(p *porter.Porter) *cobra.Command { Short: "Fix the permissions on your PORTER_HOME directory", Long: `This will reset the permissions on your PORTER_HOME directory to the least permissions required, where only the current user has permissions.`, RunE: func(cmd *cobra.Command, args []string) error { - return p.FixPermissions() + return p.FixPermissions(cmd.Context()) }, } } diff --git a/pkg/cnab/cnab-to-oci/helpers.go b/pkg/cnab/cnab-to-oci/helpers.go index 8fbc000dc..18576853f 100644 --- a/pkg/cnab/cnab-to-oci/helpers.go +++ b/pkg/cnab/cnab-to-oci/helpers.go @@ -21,7 +21,7 @@ func NewTestRegistry() *TestRegistry { return &TestRegistry{} } -func (t TestRegistry) PullBundle(ref cnab.OCIReference, insecureRegistry bool) (cnab.BundleReference, error) { +func (t TestRegistry) PullBundle(ctx context.Context, ref cnab.OCIReference, insecureRegistry bool) (cnab.BundleReference, error) { if t.MockPullBundle != nil { return t.MockPullBundle(ref, insecureRegistry) } diff --git a/pkg/cnab/cnab-to-oci/provider.go b/pkg/cnab/cnab-to-oci/provider.go index 67da61a8b..dfd1ad342 100644 --- a/pkg/cnab/cnab-to-oci/provider.go +++ b/pkg/cnab/cnab-to-oci/provider.go @@ -10,7 +10,7 @@ import ( // RegistryProvider handles talking with an OCI registry. type RegistryProvider interface { // PullBundle pulls a bundle from an OCI registry. - PullBundle(ref cnab.OCIReference, insecureRegistry bool) (cnab.BundleReference, error) + PullBundle(ctx context.Context, ref cnab.OCIReference, insecureRegistry bool) (cnab.BundleReference, error) // PushBundle pushes a bundle to an OCI registry. PushBundle(ctx context.Context, bundleRef cnab.BundleReference, insecureRegistry bool) (cnab.BundleReference, error) diff --git a/pkg/cnab/cnab-to-oci/registry.go b/pkg/cnab/cnab-to-oci/registry.go index d15e97bc8..c6a10cce1 100644 --- a/pkg/cnab/cnab-to-oci/registry.go +++ b/pkg/cnab/cnab-to-oci/registry.go @@ -20,6 +20,8 @@ import ( "github.com/google/go-containerregistry/pkg/crane" "github.com/moby/term" "github.com/opencontainers/go-digest" + "go.opentelemetry.io/otel/attribute" + "go.uber.org/zap/zapcore" ) // ErrNoContentDigest represents an error due to an image not having a @@ -45,24 +47,30 @@ func NewRegistry(c *portercontext.Context) *Registry { } // PullBundle pulls a bundle from an OCI registry. Returns the bundle, and an optional image relocation mapping, if applicable. -func (r *Registry) PullBundle(ref cnab.OCIReference, insecureRegistry bool) (cnab.BundleReference, error) { +func (r *Registry) PullBundle(ctx context.Context, ref cnab.OCIReference, insecureRegistry bool) (cnab.BundleReference, error) { + ctx, span := tracing.StartSpan(ctx, + attribute.String("reference", ref.String()), + attribute.Bool("insecure", insecureRegistry), + ) + defer span.EndSpan() + var insecureRegistries []string if insecureRegistry { reg := ref.Registry() insecureRegistries = append(insecureRegistries, reg) } - if r.Debug { + if span.ShouldLog(zapcore.DebugLevel) { msg := strings.Builder{} msg.WriteString("Pulling bundle ") msg.WriteString(ref.String()) if insecureRegistry { msg.WriteString(" with --insecure-registry") } - fmt.Fprintln(r.Err, msg.String()) + span.Debug(msg.String()) } - bun, reloMap, digest, err := remotes.Pull(context.Background(), ref.Named, r.createResolver(insecureRegistries)) + bun, reloMap, digest, err := remotes.Pull(ctx, ref.Named, r.createResolver(insecureRegistries)) if err != nil { return cnab.BundleReference{}, fmt.Errorf("unable to pull bundle: %w", err) } diff --git a/pkg/cnab/provider/action.go b/pkg/cnab/provider/action.go index 9e3c603e4..8cd575330 100644 --- a/pkg/cnab/provider/action.go +++ b/pkg/cnab/provider/action.go @@ -1,12 +1,12 @@ package cnabprovider import ( - "bytes" "context" "encoding/json" "errors" "fmt" "sort" + "strings" "get.porter.sh/porter/pkg/cnab" "get.porter.sh/porter/pkg/config" @@ -17,6 +17,7 @@ import ( "github.com/cnabio/cnab-go/driver" "github.com/hashicorp/go-multierror" "go.opentelemetry.io/otel/attribute" + "go.uber.org/zap/zapcore" ) // Shared arguments for all CNAB actions @@ -170,7 +171,7 @@ func (r *Runtime) Execute(ctx context.Context, args ActionArguments) error { } } - r.printDebugInfo(b, creds, args.Params) + r.printDebugInfo(ctx, b, creds, args.Params) opResult, result, err := a.Run(currentRun.ToCNAB(), creds.ToCNAB(), r.ApplyConfig(ctx, args)...) @@ -301,28 +302,30 @@ func (r *Runtime) appendFailedResult(ctx context.Context, opErr error, run stora return multierror.Append(opErr, resultErr).ErrorOrNil() } -func (r *Runtime) printDebugInfo(b cnab.ExtendedBundle, creds secrets.Set, params map[string]interface{}) { - if r.Debug { - dump := &bytes.Buffer{} +func (r *Runtime) printDebugInfo(ctx context.Context, b cnab.ExtendedBundle, creds secrets.Set, params map[string]interface{}) { + log := tracing.LoggerFromContext(ctx) + + if log.ShouldLog(zapcore.DebugLevel) { + var dump strings.Builder secrets := make([]string, 0, len(params)+len(creds)) - fmt.Fprintf(dump, "params:\n") + dump.WriteString("params:\n") for k, v := range params { if b.IsSensitiveParameter(k) { // TODO(carolynvs): When we consolidate our conversion logic of parameters into strings, let's use it here. // https://github.com/cnabio/cnab-go/issues/270 secrets = append(secrets, fmt.Sprintf("%v", v)) } - fmt.Fprintf(dump, " - %s: %v\n", k, v) + dump.WriteString(fmt.Sprintf(" - %s: %v\n", k, v)) } - fmt.Fprintf(dump, "creds:\n") + dump.WriteString("creds:\n") for k, v := range creds { secrets = append(secrets, fmt.Sprintf("%v", v)) - fmt.Fprintf(dump, " - %s: %v\n", k, v) + dump.WriteString(fmt.Sprintf(" - %s: %v\n", k, v)) } r.Context.SetSensitiveValues(secrets) - fmt.Fprintln(r.Err, dump.String()) + log.Debug(dump.String()) } } diff --git a/pkg/config/config.go b/pkg/config/config.go index a4f7be94c..692bb641a 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -220,24 +220,23 @@ func (c *Config) SetPorterPath(path string) { c.porterPath = path } -func (c *Config) GetPorterPath() (string, error) { +func (c *Config) GetPorterPath(ctx context.Context) (string, error) { if c.porterPath != "" { return c.porterPath, nil } + log := tracing.LoggerFromContext(ctx) porterPath, err := getExecutable() if err != nil { - return "", fmt.Errorf("could not get path to the executing porter binary: %w", err) + return "", log.Error(fmt.Errorf("could not get path to the executing porter binary: %w", err)) } // We try to resolve back to the original location hardPath, err := evalSymlinks(porterPath) if err != nil { // if we have trouble resolving symlinks, skip trying to help people who used symlinks - fmt.Fprintln(c.Err, fmt.Errorf("WARNING could not resolve %s for symbolic links\n: %w", porterPath, err)) + log.Error(fmt.Errorf("WARNING could not resolve %s for symbolic links: %w", porterPath, err)) } else if hardPath != porterPath { - if c.Debug { - fmt.Fprintf(c.Err, "Resolved porter binary from %s to %s\n", porterPath, hardPath) - } + log.Debugf("Resolved porter binary from %s to %s", porterPath, hardPath) porterPath = hardPath } diff --git a/pkg/exec/builder/action.go b/pkg/exec/builder/action.go index 830a95dc8..07f755e07 100644 --- a/pkg/exec/builder/action.go +++ b/pkg/exec/builder/action.go @@ -2,10 +2,12 @@ package builder import ( "bufio" + "context" "fmt" "io/ioutil" "get.porter.sh/porter/pkg/portercontext" + "get.porter.sh/porter/pkg/tracing" "get.porter.sh/porter/pkg/yaml" ) @@ -73,18 +75,19 @@ func unmarshalActionMap(actionMap map[string][]interface{}, builder BuildableAct // err := yaml.Unmarshal(contents, &action) // return &action, err // }) -func LoadAction(cxt *portercontext.Context, commandFile string, unmarshal func([]byte) (interface{}, error)) error { - contents, err := readInputFromStdinOrFile(cxt, commandFile) +func LoadAction(ctx context.Context, porterCtx *portercontext.Context, commandFile string, unmarshal func([]byte) (interface{}, error)) error { + //lint:ignore SA4006 ignore unused ctx for now + ctx, span := tracing.StartSpan(ctx) + defer span.EndSpan() + + contents, err := readInputFromStdinOrFile(porterCtx, commandFile) if err != nil { - return err + return span.Error(err) } - result, err := unmarshal(contents) - if cxt.Debug { - fmt.Fprintf(cxt.Err, "DEBUG Parsed Input:\n%#v\n", result) - } + _, err = unmarshal(contents) if err != nil { - return fmt.Errorf("could not unmarshal input:\n %s: %w", string(contents), err) + return span.Error(fmt.Errorf("could not unmarshal input:\n %s: %w", string(contents), err)) } return nil diff --git a/pkg/exec/exec_test.go b/pkg/exec/exec_test.go index 5078b16d7..5c06072dd 100644 --- a/pkg/exec/exec_test.go +++ b/pkg/exec/exec_test.go @@ -2,6 +2,7 @@ package exec import ( "bytes" + "context" "fmt" "io/ioutil" "sort" @@ -9,7 +10,7 @@ import ( "get.porter.sh/porter/pkg/exec/builder" "get.porter.sh/porter/pkg/test" - yaml "get.porter.sh/porter/pkg/yaml" + "get.porter.sh/porter/pkg/yaml" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -38,6 +39,8 @@ func TestAction_UnmarshalYAML(t *testing.T) { } func TestMixin_ExecuteCommand(t *testing.T) { + ctx := context.Background() + step := Step{ Instruction: Instruction{ Command: "bash", @@ -56,7 +59,7 @@ func TestMixin_ExecuteCommand(t *testing.T) { m.Setenv(test.ExpectedCommandEnv, `bash -c echo Hello World`) - err := m.Execute(ExecuteOptions{}) + err := m.Execute(ctx, ExecuteOptions{}) require.NoError(t, err) } @@ -76,7 +79,7 @@ func TestMixin_ErrorHandling(t *testing.T) { for _, tc := range testcases { t.Run(tc.name, func(t *testing.T) { - + ctx := context.Background() step := Step{ Instruction: Instruction{ Command: "bash", @@ -97,7 +100,7 @@ func TestMixin_ErrorHandling(t *testing.T) { m.Setenv(test.ExpectedCommandExitCodeEnv, "1") m.Setenv(test.ExpectedCommandErrorEnv, "thing already exists") - err := m.Execute(ExecuteOptions{}) + err := m.Execute(ctx, ExecuteOptions{}) if tc.wantError == "" { require.NoError(t, err) } else { @@ -109,10 +112,11 @@ func TestMixin_ErrorHandling(t *testing.T) { } func TestMixin_Install(t *testing.T) { + ctx := context.Background() h := NewTestMixin(t) h.TestContext.AddTestDirectory("testdata", "testdata") - action, err := h.loadAction("testdata/install-input.yaml") + action, err := h.loadAction(ctx, "testdata/install-input.yaml") require.NoError(t, err) assert.Len(t, action.Steps, 1) @@ -123,10 +127,11 @@ func TestMixin_Install(t *testing.T) { } func TestMixin_Upgrade(t *testing.T) { + ctx := context.Background() h := NewTestMixin(t) h.TestContext.AddTestDirectory("testdata", "testdata") - action, err := h.loadAction("testdata/upgrade-input.yaml") + action, err := h.loadAction(ctx, "testdata/upgrade-input.yaml") require.NoError(t, err) assert.Len(t, action.Steps, 1) @@ -137,10 +142,11 @@ func TestMixin_Upgrade(t *testing.T) { } func TestMixin_CustomAction(t *testing.T) { + ctx := context.Background() h := NewTestMixin(t) h.TestContext.AddTestDirectory("testdata", "testdata") - action, err := h.loadAction("testdata/invoke-input.yaml") + action, err := h.loadAction(ctx, "testdata/invoke-input.yaml") require.NoError(t, err) assert.Len(t, action.Steps, 1) @@ -151,10 +157,11 @@ func TestMixin_CustomAction(t *testing.T) { } func TestMixin_Uninstall(t *testing.T) { + ctx := context.Background() h := NewTestMixin(t) h.TestContext.AddTestDirectory("testdata", "testdata") - action, err := h.loadAction("testdata/uninstall-input.yaml") + action, err := h.loadAction(ctx, "testdata/uninstall-input.yaml") require.NoError(t, err) assert.Len(t, action.Steps, 1) @@ -163,6 +170,7 @@ func TestMixin_Uninstall(t *testing.T) { } func TestMixin_SuffixArgs(t *testing.T) { + ctx := context.Background() b, err := ioutil.ReadFile("testdata/suffix-args-input.yaml") require.NoError(t, err, "ReadFile failed") @@ -175,6 +183,6 @@ func TestMixin_SuffixArgs(t *testing.T) { m.Setenv(test.ExpectedCommandEnv, `docker build --tag getporter/porter-hello:latest .`) - err = m.Execute(ExecuteOptions{}) + err = m.Execute(ctx, ExecuteOptions{}) require.NoError(t, err) } diff --git a/pkg/exec/execute.go b/pkg/exec/execute.go index f2a38783a..0f735f6a8 100644 --- a/pkg/exec/execute.go +++ b/pkg/exec/execute.go @@ -1,8 +1,10 @@ package exec import ( + "context" + "get.porter.sh/porter/pkg/exec/builder" - yaml "get.porter.sh/porter/pkg/yaml" + "get.porter.sh/porter/pkg/yaml" ) // ExecOptions represent the options for any exec command @@ -10,17 +12,17 @@ type ExecuteOptions struct { File string } -func (m *Mixin) loadAction(commandFile string) (*Action, error) { +func (m *Mixin) loadAction(ctx context.Context, commandFile string) (*Action, error) { var action Action - err := builder.LoadAction(m.Context, commandFile, func(contents []byte) (interface{}, error) { + err := builder.LoadAction(ctx, m.Context, commandFile, func(contents []byte) (interface{}, error) { err := yaml.Unmarshal(contents, &action) return &action, err }) return &action, err } -func (m *Mixin) Execute(opts ExecuteOptions) error { - action, err := m.loadAction(opts.File) +func (m *Mixin) Execute(ctx context.Context, opts ExecuteOptions) error { + action, err := m.loadAction(ctx, opts.File) if err != nil { return err } diff --git a/pkg/exec/execute_test.go b/pkg/exec/execute_test.go index f03a44ef5..a99c15a20 100644 --- a/pkg/exec/execute_test.go +++ b/pkg/exec/execute_test.go @@ -2,6 +2,7 @@ package exec import ( "bytes" + "context" "io/ioutil" "testing" @@ -12,6 +13,7 @@ import ( ) func TestMixin_Execute(t *testing.T) { + ctx := context.Background() m := NewTestMixin(t) err := m.FileSystem.WriteFile("config.txt", []byte("abc123"), pkg.FileModeWritable) @@ -23,7 +25,7 @@ func TestMixin_Execute(t *testing.T) { m.Setenv(test.ExpectedCommandEnv, "foo") - err = m.Execute(ExecuteOptions{}) + err = m.Execute(ctx, ExecuteOptions{}) require.NoError(t, err, "Execute should not have returned an error") exists, _ := m.FileSystem.Exists("/cnab/app/porter/outputs/file") diff --git a/pkg/exec/lint.go b/pkg/exec/lint.go index 5c50d6844..6d18ca923 100644 --- a/pkg/exec/lint.go +++ b/pkg/exec/lint.go @@ -1,6 +1,7 @@ package exec import ( + "context" "fmt" "strings" @@ -26,10 +27,10 @@ const ( CodeBashCArgMissingQuotes linter.Code = "exec-101" ) -func (m *Mixin) Lint() (linter.Results, error) { +func (m *Mixin) Lint(ctx context.Context) (linter.Results, error) { var input BuildInput - err := builder.LoadAction(m.Context, "", func(contents []byte) (interface{}, error) { + err := builder.LoadAction(ctx, m.Context, "", func(contents []byte) (interface{}, error) { err := yaml.Unmarshal(contents, &input) return &input, err }) @@ -111,8 +112,8 @@ exec: return results, nil } -func (m *Mixin) PrintLintResults() error { - results, err := m.Lint() +func (m *Mixin) PrintLintResults(ctx context.Context) error { + results, err := m.Lint(ctx) if err != nil { return err } diff --git a/pkg/exec/lint_test.go b/pkg/exec/lint_test.go index d57f77788..c8bed6c3e 100644 --- a/pkg/exec/lint_test.go +++ b/pkg/exec/lint_test.go @@ -2,6 +2,7 @@ package exec import ( "bytes" + "context" "io/ioutil" "testing" @@ -11,13 +12,14 @@ import ( ) func TestMixin_Lint(t *testing.T) { + ctx := context.Background() m := NewTestMixin(t) input, err := ioutil.ReadFile("testdata/lint-input.yaml") require.NoError(t, err, "could not read lint testdata") m.In = bytes.NewReader(input) - results, err := m.Lint() + results, err := m.Lint(ctx) require.NoError(t, err, "Lint failed") assert.Len(t, results, 2, "Unexpected number of lint results generated") diff --git a/pkg/linter/linter.go b/pkg/linter/linter.go index c9ac92e49..aafce889a 100644 --- a/pkg/linter/linter.go +++ b/pkg/linter/linter.go @@ -10,6 +10,7 @@ import ( "get.porter.sh/porter/pkg/mixin/query" "get.porter.sh/porter/pkg/pkgmgmt" "get.porter.sh/porter/pkg/portercontext" + "get.porter.sh/porter/pkg/tracing" "github.com/dustin/go-humanize" ) @@ -155,14 +156,14 @@ func (l *Linter) Lint(ctx context.Context, m *manifest.Manifest) (Results, error // TODO: perform any porter level linting // e.g. metadata, credentials, properties, outputs, dependencies, etc - if l.Debug { - fmt.Fprintln(l.Err, "Running linters for each mixin used in the manifest...") - } + ctx, span := tracing.StartSpan(ctx) + defer span.EndSpan() + span.Debug("Running linters for each mixin used in the manifest...") q := query.New(l.Context, l.Mixins) responses, err := q.Execute(ctx, "lint", query.NewManifestGenerator(m)) if err != nil { - return nil, err + return nil, span.Error(err) } var results Results @@ -170,7 +171,7 @@ func (l *Linter) Lint(ctx context.Context, m *manifest.Manifest) (Results, error var r Results err = json.Unmarshal([]byte(response), &r) if err != nil { - return nil, fmt.Errorf("unable to parse lint response from mixin %q: %w", mixin, err) + return nil, span.Error(fmt.Errorf("unable to parse lint response from mixin %q: %w", mixin, err)) } results = append(results, r...) diff --git a/pkg/mixin/pkgmgmt.go b/pkg/mixin/pkgmgmt.go index d505b8fec..a51f76f4d 100644 --- a/pkg/mixin/pkgmgmt.go +++ b/pkg/mixin/pkgmgmt.go @@ -9,6 +9,8 @@ import ( "get.porter.sh/porter/pkg/config" "get.porter.sh/porter/pkg/pkgmgmt" "get.porter.sh/porter/pkg/pkgmgmt/client" + "get.porter.sh/porter/pkg/tracing" + "go.uber.org/zap/zapcore" ) const ( @@ -47,6 +49,8 @@ func (c *PackageManager) PreRunMixinCommandHandler(command string, cmd *exec.Cmd } func (c *PackageManager) GetSchema(ctx context.Context, name string) (string, error) { + log := tracing.LoggerFromContext(ctx) + mixinDir, err := c.GetPackageDir(name) if err != nil { return "", err @@ -58,7 +62,7 @@ func (c *PackageManager) GetSchema(ctx context.Context, name string) (string, er mixinSchema := &bytes.Buffer{} mixinContext := *c.Context mixinContext.Out = mixinSchema - if !c.Debug { + if !log.ShouldLog(zapcore.DebugLevel) { mixinContext.Err = ioutil.Discard } r.Context = &mixinContext diff --git a/pkg/mixin/pkgmgmt_test.go b/pkg/mixin/pkgmgmt_test.go index 97b1d0c8a..df81a0179 100644 --- a/pkg/mixin/pkgmgmt_test.go +++ b/pkg/mixin/pkgmgmt_test.go @@ -28,7 +28,6 @@ func TestRunner_BuildCommand(t *testing.T) { t.Run(tc.name, func(t *testing.T) { ctx := context.Background() r := client.NewTestRunner(t, "exec", "mixins", false) - r.Debug = false r.Setenv(test.ExpectedCommandEnv, tc.wantCommand) mgr := PackageManager{} diff --git a/pkg/mixin/query/query.go b/pkg/mixin/query/query.go index 118498795..0846af451 100644 --- a/pkg/mixin/query/query.go +++ b/pkg/mixin/query/query.go @@ -8,6 +8,7 @@ import ( "get.porter.sh/porter/pkg/pkgmgmt" "get.porter.sh/porter/pkg/portercontext" + "get.porter.sh/porter/pkg/tracing" "github.com/hashicorp/go-multierror" "golang.org/x/sync/errgroup" ) @@ -49,6 +50,9 @@ type MixinInputGenerator interface { // For example, the ManifestGenerator will iterate over the mixins in a manifest and send // them their config and the steps associated with their mixin. func (q *MixinQuery) Execute(ctx context.Context, cmd string, inputGenerator MixinInputGenerator) (map[string]string, error) { + ctx, span := tracing.StartSpan(ctx) + defer span.EndSpan() + mixinNames := inputGenerator.ListMixins() results := make(map[string]string, len(mixinNames)) type queryResponse struct { @@ -116,15 +120,13 @@ func (q *MixinQuery) Execute(ctx context.Context, cmd string, inputGenerator Mix if runErr != nil { if q.RequireAllMixinResponses { - return nil, runErr + return nil, span.Error(runErr) } // This is a debug because we expect not all mixins to implement some // optional commands, like lint and don't want to print their error // message when we query them with a command they don't support. - if q.Debug { - fmt.Fprintln(q.Err, fmt.Errorf("not all mixins responded successfully: %w", runErr)) - } + span.Debugf(fmt.Errorf("not all mixins responded successfully: %w", runErr).Error()) } return results, nil diff --git a/pkg/pkgmgmt/client/delete.go b/pkg/pkgmgmt/client/delete.go index 849770b20..bc9f41524 100644 --- a/pkg/pkgmgmt/client/delete.go +++ b/pkg/pkgmgmt/client/delete.go @@ -1,39 +1,44 @@ package client import ( + "context" "fmt" "path/filepath" "get.porter.sh/porter/pkg/pkgmgmt" + "get.porter.sh/porter/pkg/tracing" ) -func (fs *FileSystem) Uninstall(opts pkgmgmt.UninstallOptions) error { +func (fs *FileSystem) Uninstall(ctx context.Context, opts pkgmgmt.UninstallOptions) error { + ctx, span := tracing.StartSpan(ctx) + defer span.EndSpan() + if opts.Name != "" { - return fs.uninstallByName(opts.Name) + return fs.uninstallByName(ctx, opts.Name) } - return fmt.Errorf("No %s name was provided to uninstall", fs.PackageType) + return span.Error(fmt.Errorf("No %s name was provided to uninstall", fs.PackageType)) } -func (fs *FileSystem) uninstallByName(name string) error { +func (fs *FileSystem) uninstallByName(ctx context.Context, name string) error { + log := tracing.LoggerFromContext(ctx) + parentDir, err := fs.GetPackagesDir() if err != nil { - return err + return log.Error(err) } pkgDir := filepath.Join(parentDir, name) exists, _ := fs.FileSystem.Exists(pkgDir) if exists { err = fs.FileSystem.RemoveAll(pkgDir) if err != nil { - return fmt.Errorf("could not remove %s directory %q: %w", fs.PackageType, pkgDir, err) + return log.Error(fmt.Errorf("could not remove %s directory %q: %w", fs.PackageType, pkgDir, err)) } return nil } - if fs.Debug { - fmt.Fprintf(fs.Err, "Unable to find requested %s %s\n", fs.PackageType, name) - } + log.Debugf("Unable to find requested %s %s\n", fs.PackageType, name) return nil } diff --git a/pkg/pkgmgmt/client/delete_test.go b/pkg/pkgmgmt/client/delete_test.go index 550430311..a0db94d9c 100644 --- a/pkg/pkgmgmt/client/delete_test.go +++ b/pkg/pkgmgmt/client/delete_test.go @@ -1,6 +1,7 @@ package client import ( + "context" "path" "testing" @@ -10,6 +11,7 @@ import ( ) func TestFileSystem_Delete_DeletePackage(t *testing.T) { + ctx := context.Background() c := config.NewTestConfig(t) p := NewFileSystem(c.Config, "packages") @@ -20,7 +22,7 @@ func TestFileSystem_Delete_DeletePackage(t *testing.T) { Name: "mixxin", } - err := p.Uninstall(opts) + err := p.Uninstall(ctx, opts) assert.Nil(t, err) diff --git a/pkg/pkgmgmt/client/filesystem.go b/pkg/pkgmgmt/client/filesystem.go index f24e7ea23..d2740818c 100644 --- a/pkg/pkgmgmt/client/filesystem.go +++ b/pkg/pkgmgmt/client/filesystem.go @@ -13,6 +13,7 @@ import ( "get.porter.sh/porter/pkg/portercontext" "get.porter.sh/porter/pkg/tracing" "go.opentelemetry.io/otel/attribute" + "go.uber.org/zap/zapcore" ) var _ pkgmgmt.PackageManager = &FileSystem{} @@ -73,7 +74,7 @@ func (fs *FileSystem) GetMetadata(ctx context.Context, name string) (pkgmgmt.Pac pkgDir, err := fs.GetPackageDir(name) if err != nil { - return nil, err + return nil, span.Error(err) } r := NewRunner(name, pkgDir, false) @@ -81,7 +82,7 @@ func (fs *FileSystem) GetMetadata(ctx context.Context, name string) (pkgmgmt.Pac jsonB := &bytes.Buffer{} pkgContext := *fs.Context pkgContext.Out = jsonB - if !fs.Debug { + if span.ShouldLog(zapcore.DebugLevel) { pkgContext.Err = ioutil.Discard } r.Context = &pkgContext @@ -89,13 +90,13 @@ func (fs *FileSystem) GetMetadata(ctx context.Context, name string) (pkgmgmt.Pac cmd := pkgmgmt.CommandOptions{Command: "version --output json", PreRun: fs.PreRun} err = r.Run(ctx, cmd) if err != nil { - return nil, err + return nil, span.Error(err) } result := fs.BuildMetadata() err = json.Unmarshal(jsonB.Bytes(), &result) if err != nil { - return nil, err + return nil, span.Error(err) } return result, nil diff --git a/pkg/pkgmgmt/client/helpers.go b/pkg/pkgmgmt/client/helpers.go index 0a1c54ab6..a55ea770d 100644 --- a/pkg/pkgmgmt/client/helpers.go +++ b/pkg/pkgmgmt/client/helpers.go @@ -63,7 +63,7 @@ func (p *TestPackageManager) Install(ctx context.Context, opts pkgmgmt.InstallOp return nil } -func (p *TestPackageManager) Uninstall(o pkgmgmt.UninstallOptions) error { +func (p *TestPackageManager) Uninstall(ctx context.Context, opts pkgmgmt.UninstallOptions) error { // do nothing return nil } diff --git a/pkg/pkgmgmt/client/install.go b/pkg/pkgmgmt/client/install.go index 4bfc9fa84..94304c0b4 100644 --- a/pkg/pkgmgmt/client/install.go +++ b/pkg/pkgmgmt/client/install.go @@ -127,7 +127,7 @@ func (fs *FileSystem) InstallFromFeedURL(ctx context.Context, opts pkgmgmt.Insta } searchFeed := feed.NewMixinFeed(fs.Context) - err = searchFeed.Load(feedPath) + err = searchFeed.Load(ctx, feedPath) if err != nil { return err } diff --git a/pkg/pkgmgmt/client/runner.go b/pkg/pkgmgmt/client/runner.go index 5ea508d28..76e6c3efb 100644 --- a/pkg/pkgmgmt/client/runner.go +++ b/pkg/pkgmgmt/client/runner.go @@ -10,6 +10,9 @@ import ( "get.porter.sh/porter/pkg/pkgmgmt" "get.porter.sh/porter/pkg/portercontext" + "get.porter.sh/porter/pkg/tracing" + "go.opentelemetry.io/otel/attribute" + "go.uber.org/zap/zapcore" ) type Runner struct { @@ -48,12 +51,13 @@ func (r *Runner) Validate() error { } func (r *Runner) Run(ctx context.Context, commandOpts pkgmgmt.CommandOptions) error { - if r.Debug { - fmt.Fprintf(r.Err, "DEBUG name: %s\n", r.pkgName) - fmt.Fprintf(r.Err, "DEBUG pkgDir: %s\n", r.pkgDir) - fmt.Fprintf(r.Err, "DEBUG file: %s\n", commandOpts.File) - fmt.Fprintf(r.Err, "DEBUG stdin:\n%s\n", commandOpts.Input) - } + ctx, span := tracing.StartSpan(ctx, + attribute.String("name", r.pkgName), + attribute.String("pkgDir", r.pkgDir), + attribute.String("file", commandOpts.File), + attribute.String("stdin", commandOpts.Input), + ) + defer span.EndSpan() pkgPath := r.getExecutablePath() cmdArgs := strings.Split(commandOpts.Command, " ") @@ -72,14 +76,14 @@ func (r *Runner) Run(ctx context.Context, commandOpts pkgmgmt.CommandOptions) er cmd.Args = append(cmd.Args, "-f", commandOpts.File) } - if r.Debug { + if span.ShouldLog(zapcore.DebugLevel) { cmd.Args = append(cmd.Args, "--debug") } if commandOpts.Input != "" { stdin, err := cmd.StdinPipe() if err != nil { - return err + return span.Error(err) } go func() { defer stdin.Close() @@ -88,16 +92,14 @@ func (r *Runner) Run(ctx context.Context, commandOpts pkgmgmt.CommandOptions) er } prettyCmd := fmt.Sprintf("%s%s", cmd.Dir, strings.Join(cmd.Args, " ")) - if r.Debug { - fmt.Fprintln(r.Err, prettyCmd) - } + span.SetAttributes(attribute.String("command", prettyCmd)) err := cmd.Start() if err != nil { - return fmt.Errorf("could not run package command %s: %w", prettyCmd, err) + return span.Error(fmt.Errorf("could not run package command %s: %w", prettyCmd, err)) } - return cmd.Wait() + return span.Error(cmd.Wait()) } func (r *Runner) getExecutablePath() string { diff --git a/pkg/pkgmgmt/feed/generate.go b/pkg/pkgmgmt/feed/generate.go index 5040fe673..affd89ee1 100644 --- a/pkg/pkgmgmt/feed/generate.go +++ b/pkg/pkgmgmt/feed/generate.go @@ -1,6 +1,7 @@ package feed import ( + "context" "fmt" "os" "regexp" @@ -9,6 +10,7 @@ import ( "get.porter.sh/porter/pkg" "get.porter.sh/porter/pkg/portercontext" + "get.porter.sh/porter/pkg/tracing" "github.com/Masterminds/semver/v3" "github.com/cbroglie/mustache" ) @@ -48,13 +50,16 @@ func (o *GenerateOptions) ValidateTemplateFile(cxt *portercontext.Context) error return nil } -func (feed *MixinFeed) Generate(opts GenerateOptions) error { +func (feed *MixinFeed) Generate(ctx context.Context, opts GenerateOptions) error { + ctx, span := tracing.StartSpan(ctx) + defer span.EndSpan() + existingFeed, err := feed.FileSystem.Exists(opts.AtomFile) if err != nil { return err } if existingFeed { - err := feed.Load(opts.AtomFile) + err := feed.Load(ctx, opts.AtomFile) if err != nil { return err } @@ -112,11 +117,11 @@ func (feed *MixinFeed) Generate(opts GenerateOptions) error { }) if err != nil { - return fmt.Errorf("failed to traverse the %s directory: %w", opts.SearchDirectory, err) + return span.Error(fmt.Errorf("failed to traverse the %s directory: %w", opts.SearchDirectory, err)) } if len(feed.Index) == 0 { - return fmt.Errorf("no mixin binaries found in %s matching the regex %q", opts.SearchDirectory, mixinRegex) + return span.Error(fmt.Errorf("no mixin binaries found in %s matching the regex %q", opts.SearchDirectory, mixinRegex)) } return nil diff --git a/pkg/pkgmgmt/feed/generate_test.go b/pkg/pkgmgmt/feed/generate_test.go index 96c25bd35..2b9db8be9 100644 --- a/pkg/pkgmgmt/feed/generate_test.go +++ b/pkg/pkgmgmt/feed/generate_test.go @@ -1,6 +1,7 @@ package feed import ( + "context" "fmt" "io/ioutil" "sort" @@ -13,6 +14,7 @@ import ( ) func TestGenerate(t *testing.T) { + ctx := context.Background() tc := portercontext.NewTestContext(t) tc.AddTestFile("testdata/atom-template.xml", "template.xml") @@ -72,7 +74,7 @@ func TestGenerate(t *testing.T) { TemplateFile: "template.xml", } f := NewMixinFeed(tc.Context) - err := f.Generate(opts) + err := f.Generate(ctx, opts) require.NoError(t, err) err = f.Save(opts) require.NoError(t, err) @@ -133,13 +135,14 @@ func TestGenerate_RegexMatch(t *testing.T) { for _, tc := range testcases { t.Run(tc.name, func(t *testing.T) { - ctx := portercontext.NewTestContext(t) - ctx.AddTestFile("testdata/atom-template.xml", "template.xml") + ctx := context.Background() + porterCtx := portercontext.NewTestContext(t) + porterCtx.AddTestFile("testdata/atom-template.xml", "template.xml") if tc.mixinName != "" { - ctx.FileSystem.Create(fmt.Sprintf("bin/v1.2.3/%s-darwin-amd64", tc.mixinName)) - ctx.FileSystem.Create(fmt.Sprintf("bin/v1.2.3/%s-linux-amd64", tc.mixinName)) - ctx.FileSystem.Create(fmt.Sprintf("bin/v1.2.3/%s-windows-amd64.exe", tc.mixinName)) + porterCtx.FileSystem.Create(fmt.Sprintf("bin/v1.2.3/%s-darwin-amd64", tc.mixinName)) + porterCtx.FileSystem.Create(fmt.Sprintf("bin/v1.2.3/%s-linux-amd64", tc.mixinName)) + porterCtx.FileSystem.Create(fmt.Sprintf("bin/v1.2.3/%s-windows-amd64.exe", tc.mixinName)) } opts := GenerateOptions{ @@ -147,8 +150,8 @@ func TestGenerate_RegexMatch(t *testing.T) { SearchDirectory: "bin", TemplateFile: "template.xml", } - f := NewMixinFeed(ctx.Context) - err := f.Generate(opts) + f := NewMixinFeed(porterCtx.Context) + err := f.Generate(ctx, opts) if tc.wantError != "" { require.EqualError(t, err, tc.wantError) } else { @@ -159,6 +162,7 @@ func TestGenerate_RegexMatch(t *testing.T) { } func TestGenerate_ExistingFeed(t *testing.T) { + ctx := context.Background() tc := portercontext.NewTestContext(t) tc.AddTestFile("testdata/atom-template.xml", "template.xml") tc.AddTestFile("testdata/atom-existing.xml", "atom.xml") @@ -187,7 +191,7 @@ func TestGenerate_ExistingFeed(t *testing.T) { TemplateFile: "template.xml", } f := NewMixinFeed(tc.Context) - err := f.Generate(opts) + err := f.Generate(ctx, opts) require.NoError(t, err) err = f.Save(opts) require.NoError(t, err) @@ -204,6 +208,7 @@ func TestGenerate_ExistingFeed(t *testing.T) { } func TestGenerate_RegenerateDoesNotCreateDuplicates(t *testing.T) { + ctx := context.Background() tc := portercontext.NewTestContext(t) tc.AddTestFile("testdata/atom-template.xml", "template.xml") tc.AddTestFile("testdata/atom-existing.xml", "atom.xml") @@ -233,7 +238,7 @@ func TestGenerate_RegenerateDoesNotCreateDuplicates(t *testing.T) { } f := NewMixinFeed(tc.Context) - err := f.Generate(opts) + err := f.Generate(ctx, opts) require.NoError(t, err) err = f.Save(opts) require.NoError(t, err) @@ -241,7 +246,7 @@ func TestGenerate_RegenerateDoesNotCreateDuplicates(t *testing.T) { // Run the generation again, against the same versions, and make sure they don't insert duplicate files // This mimics what the CI does when we repeat a build, or have multiple // canary builds on the "main" branch - err = f.Generate(opts) + err = f.Generate(ctx, opts) require.NoError(t, err) err = f.Save(opts) require.NoError(t, err) diff --git a/pkg/pkgmgmt/feed/load.go b/pkg/pkgmgmt/feed/load.go index 35e93e22f..85a6a74ba 100644 --- a/pkg/pkgmgmt/feed/load.go +++ b/pkg/pkgmgmt/feed/load.go @@ -2,26 +2,31 @@ package feed import ( "bytes" + "context" "fmt" "net/url" "path" + "get.porter.sh/porter/pkg/tracing" "github.com/mmcdole/gofeed/atom" + "go.opentelemetry.io/otel/attribute" ) -func (feed *MixinFeed) Load(file string) error { +func (feed *MixinFeed) Load(ctx context.Context, file string) error { + //lint:ignore SA4006 ignore unused ctx for now + ctx, span := tracing.StartSpan(ctx) + defer span.EndSpan() + contents, err := feed.FileSystem.ReadFile(file) if err != nil { - return fmt.Errorf("error reading mixin feed at %s: %w", file, err) + return span.Error(fmt.Errorf("error reading mixin feed at %s: %w", file, err)) } p := atom.Parser{} atomFeed, err := p.Parse(bytes.NewReader(contents)) if err != nil { - if feed.Debug { - fmt.Fprintln(feed.Err, string(contents)) - } - return fmt.Errorf("error parsing the mixin feed as an atom xml file: %w", err) + return span.Error(fmt.Errorf("error parsing the mixin feed as an atom xml file: %w", err), + attribute.String("contents", string(contents))) } feed.Updated = atomFeed.UpdatedParsed @@ -34,24 +39,18 @@ func (feed *MixinFeed) Load(file string) error { fileset := &MixinFileset{} if len(entry.Categories) == 0 { - if feed.Debug { - fmt.Fprintf(feed.Err, "skipping invalid entry %s, missing category (mixin name)", entry.ID) - } + span.Debugf("skipping invalid entry %s, missing category (mixin name)", entry.ID) continue } fileset.Mixin = entry.Categories[0].Term if fileset.Mixin == "" { - if feed.Debug { - fmt.Fprintf(feed.Err, "skipping invalid entry %s, empty category (mixin name)", entry.ID) - } + span.Debugf("skipping invalid entry %s, empty category (mixin name)", entry.ID) continue } fileset.Version = entry.Content.Value if fileset.Version == "" { - if feed.Debug { - fmt.Fprintf(feed.Err, "skipping invalid entry %s, empty content (version)", entry.ID) - } + span.Debugf("skipping invalid entry %s, empty content (version)", entry.ID) continue } @@ -59,17 +58,13 @@ func (feed *MixinFeed) Load(file string) error { for _, link := range entry.Links { if link.Rel == "download" { if entry.UpdatedParsed == nil { - if feed.Debug { - fmt.Fprintf(feed.Err, "skipping invalid entry %s, invalid updated %q could not be parsed as RFC3339", entry.ID, entry.Updated) - } + span.Debugf("skipping invalid entry %s, invalid updated %q could not be parsed as RFC3339", entry.ID, entry.Updated) continue } parsedUrl, err := url.Parse(link.Href) if err != nil || link.Href == "" { - if feed.Debug { - fmt.Fprintf(feed.Err, "skipping invalid entry %s, invalid link.href %q", entry.ID, link.Href) - } + span.Debugf("skipping invalid entry %s, invalid link.href %q", entry.ID, link.Href) continue } diff --git a/pkg/pkgmgmt/pkgmgmt.go b/pkg/pkgmgmt/pkgmgmt.go index 2576a01f4..91c488906 100644 --- a/pkg/pkgmgmt/pkgmgmt.go +++ b/pkg/pkgmgmt/pkgmgmt.go @@ -15,7 +15,7 @@ type PackageManager interface { GetPackageDir(name string) (string, error) GetMetadata(ctx context.Context, name string) (PackageMetadata, error) Install(ctx context.Context, opts InstallOptions) error - Uninstall(UninstallOptions) error + Uninstall(ctx context.Context, opts UninstallOptions) error // Run a command against the installed package. Run(ctx context.Context, pkgContext *portercontext.Context, name string, commandOpts CommandOptions) error diff --git a/pkg/plugins/pluggable/connection.go b/pkg/plugins/pluggable/connection.go index 6a4866ccb..153803007 100644 --- a/pkg/plugins/pluggable/connection.go +++ b/pkg/plugins/pluggable/connection.go @@ -89,7 +89,7 @@ func (c *PluginConnection) Start(ctx context.Context, pluginCfg io.Reader) error // Create a command to run the plugin if c.key.IsInternal { - porterPath, err := c.config.GetPorterPath() + porterPath, err := c.config.GetPorterPath(ctx) if err != nil { return fmt.Errorf("could not determine the path to the porter pluginProtocol: %w", err) } diff --git a/pkg/plugins/runner.go b/pkg/plugins/runner.go index 472f423c3..19053f2e0 100644 --- a/pkg/plugins/runner.go +++ b/pkg/plugins/runner.go @@ -8,6 +8,8 @@ import ( "get.porter.sh/porter/pkg/config" "get.porter.sh/porter/pkg/portercontext" + "get.porter.sh/porter/pkg/tracing" + "go.opentelemetry.io/otel/attribute" ) type CommandOptions struct { @@ -48,18 +50,17 @@ func (r *PluginRunner) Validate() error { } func (r *PluginRunner) Run(ctx context.Context, commandOpts CommandOptions) error { - if r.Debug { - fmt.Fprintln(r.Err, "DEBUG Plugin Name: ", r.pluginName) - fmt.Fprintln(r.Err, "DEBUG Plugin Command: ", commandOpts.Command) - } + ctx, span := tracing.StartSpan(ctx, + attribute.String("name", r.pluginName), + attribute.String("partial-command", commandOpts.Command), + ) + defer span.EndSpan() pluginPath, err := config.New().GetPluginPath(r.pluginName) - if r.Debug { - fmt.Fprintln(r.Err, "DEBUG Plugin Path: ", pluginPath) - } if err != nil { - return fmt.Errorf("Failed to get plugin path for %s: %w", r.pluginName, err) + return span.Error(fmt.Errorf("Failed to get plugin path for %s: %w", r.pluginName, err)) } + span.SetAttributes(attribute.String("plugin-path", pluginPath)) cmdArgs := strings.Split(commandOpts.Command, " ") cmd := r.NewCommand(ctx, pluginPath, cmdArgs...) @@ -69,14 +70,12 @@ func (r *PluginRunner) Run(ctx context.Context, commandOpts CommandOptions) erro cmd.Stderr = r.Err prettyCmd := fmt.Sprintf("%s%s", cmd.Dir, strings.Join(cmd.Args, " ")) - if r.Debug { - fmt.Fprintln(r.Err, "DEBUG Plugin Full Command: ", prettyCmd) - } + span.SetAttributes(attribute.String("full-command", prettyCmd)) err = cmd.Start() if err != nil { - return fmt.Errorf("could not run plugin command %s: %w", prettyCmd, err) + return span.Error(fmt.Errorf("could not run plugin command %s: %w", prettyCmd, err)) } - return cmd.Wait() + return span.Error(cmd.Wait()) } diff --git a/pkg/porter/build.go b/pkg/porter/build.go index 60fdda75e..2306762a4 100644 --- a/pkg/porter/build.go +++ b/pkg/porter/build.go @@ -94,24 +94,21 @@ func (o *BuildOptions) parseCustomInputs() error { } func (p *Porter) Build(ctx context.Context, opts BuildOptions) error { - ctx, log := tracing.StartSpan(ctx) - defer log.EndSpan() + ctx, span := tracing.StartSpan(ctx) + defer span.EndSpan() opts.Apply(p.Context) - - if p.Debug { - fmt.Fprintf(p.Err, "Using %s build driver\n", p.GetBuildDriver()) - } + span.Debugf("Using %s build driver", p.GetBuildDriver()) // Start with a fresh .cnab directory before building err := p.FileSystem.RemoveAll(build.LOCAL_CNAB) if err != nil { - return fmt.Errorf("could not cleanup generated .cnab directory before building: %w", err) + return span.Error(fmt.Errorf("could not cleanup generated .cnab directory before building: %w", err)) } // Generate Porter's canonical version of the user-provided manifest if err := p.generateInternalManifest(opts); err != nil { - return fmt.Errorf("unable to generate manifest: %w", err) + return span.Error(fmt.Errorf("unable to generate manifest: %w", err)) } m, err := manifest.LoadManifestFrom(ctx, p.Config, build.LOCAL_MANIFEST) @@ -137,23 +134,23 @@ func (p *Porter) Build(ctx context.Context, opts BuildOptions) error { // to a registry. The bundle.json will need to be updated after publishing // and provided just-in-time during bundle execution. if err := p.buildBundle(ctx, m, ""); err != nil { - return fmt.Errorf("unable to build bundle: %w", err) + return span.Error(fmt.Errorf("unable to build bundle: %w", err)) } generator := build.NewDockerfileGenerator(p.Config, m, p.Templates, p.Mixins) if err := generator.PrepareFilesystem(); err != nil { - return fmt.Errorf("unable to copy run script, runtimes or mixins: %s", err) + return span.Error(fmt.Errorf("unable to copy run script, runtimes or mixins: %s", err)) } if err := generator.GenerateDockerFile(ctx); err != nil { - return fmt.Errorf("unable to generate Dockerfile: %s", err) + return span.Error(fmt.Errorf("unable to generate Dockerfile: %s", err)) } builder := p.GetBuilder(ctx) err = builder.BuildInvocationImage(ctx, m, opts.BuildImageOptions) if err != nil { - return fmt.Errorf("unable to build CNAB invocation image: %w", err) + return span.Error(fmt.Errorf("unable to build CNAB invocation image: %w", err)) } return nil diff --git a/pkg/porter/copy.go b/pkg/porter/copy.go index cd5f8bc03..dd5e31b91 100644 --- a/pkg/porter/copy.go +++ b/pkg/porter/copy.go @@ -7,6 +7,8 @@ import ( "strings" "get.porter.sh/porter/pkg/cnab" + "get.porter.sh/porter/pkg/tracing" + "go.opentelemetry.io/otel/attribute" ) type CopyOpts struct { @@ -51,22 +53,28 @@ func generateNewBundleRef(source cnab.OCIReference, dest string) (cnab.OCIRefere } // CopyBundle copies a bundle from one repository to another -func (p *Porter) CopyBundle(c *CopyOpts) error { +func (p *Porter) CopyBundle(ctx context.Context, c *CopyOpts) error { + ctx, span := tracing.StartSpan(ctx, + attribute.String("source", c.sourceRef.String()), + attribute.String("destination", c.Destination), + ) + defer span.EndSpan() + destinationRef, err := generateNewBundleRef(c.sourceRef, c.Destination) if err != nil { - return err + return span.Error(err) } - fmt.Fprintf(p.Out, "Beginning bundle copy to %s. This may take some time.\n", destinationRef) - bunRef, err := p.Registry.PullBundle(c.sourceRef, c.InsecureRegistry) + span.Infof("Beginning bundle copy to %s. This may take some time.", destinationRef) + bunRef, err := p.Registry.PullBundle(ctx, c.sourceRef, c.InsecureRegistry) if err != nil { - return fmt.Errorf("unable to pull bundle before copying: %w", err) + return span.Error(fmt.Errorf("unable to pull bundle before copying: %w", err)) } bunRef.Reference = destinationRef _, err = p.Registry.PushBundle(context.Background(), bunRef, c.InsecureRegistry) if err != nil { - return fmt.Errorf("unable to copy bundle to new location: %w", err) + return span.Error(fmt.Errorf("unable to copy bundle to new location: %w", err)) } return nil } diff --git a/pkg/porter/credentials.go b/pkg/porter/credentials.go index 1f97c1db7..9593cbfe8 100644 --- a/pkg/porter/credentials.go +++ b/pkg/porter/credentials.go @@ -13,8 +13,10 @@ import ( "get.porter.sh/porter/pkg/generator" "get.porter.sh/porter/pkg/printer" "get.porter.sh/porter/pkg/storage" + "get.porter.sh/porter/pkg/tracing" dtprinter "github.com/carolynvs/datetime-printer" "github.com/olekukonko/tablewriter" + "go.opentelemetry.io/otel/attribute" ) // CredentialShowOptions represent options for Porter's credential show command @@ -42,6 +44,9 @@ func (p *Porter) ListCredentials(ctx context.Context, opts ListOptions) ([]stora // PrintCredentials prints saved credential sets. func (p *Porter) PrintCredentials(ctx context.Context, opts ListOptions) error { + ctx, span := tracing.StartSpan(ctx) + defer span.EndSpan() + creds, err := p.ListCredentials(ctx, opts) if err != nil { return err @@ -70,7 +75,7 @@ func (p *Porter) PrintCredentials(ctx context.Context, opts ListOptions) error { return printer.PrintTable(p.Out, creds, printCredRow, "NAMESPACE", "NAME", "MODIFIED") default: - return fmt.Errorf("invalid format: %s", opts.Format) + return span.Error(fmt.Errorf("invalid format: %s", opts.Format)) } } @@ -110,6 +115,10 @@ func (o *CredentialOptions) validateCredName(args []string) error { // a silent build, based on the opts.Silent flag, or interactive using a survey. Returns an // error if unable to generate credentials func (p *Porter) GenerateCredentials(ctx context.Context, opts CredentialOptions) error { + ctx, span := tracing.StartSpan(ctx, + attribute.String("reference", opts.Reference)) + defer span.EndSpan() + bundleRef, err := p.resolveBundleReference(ctx, &opts.BundleActionOptions) if err != nil { return err @@ -128,12 +137,12 @@ func (p *Porter) GenerateCredentials(ctx context.Context, opts CredentialOptions }, Credentials: bundleRef.Definition.Credentials, } - fmt.Fprintf(p.Out, "Generating new credential %s from bundle %s\n", genOpts.Name, bundleRef.Definition.Name) - fmt.Fprintf(p.Out, "==> %d credentials required for bundle %s\n", len(genOpts.Credentials), bundleRef.Definition.Name) + span.Infof("Generating new credential %s from bundle %s\n", genOpts.Name, bundleRef.Definition.Name) + span.Infof("==> %d credentials required for bundle %s\n", len(genOpts.Credentials), bundleRef.Definition.Name) cs, err := generator.GenerateCredentials(genOpts) if err != nil { - return fmt.Errorf("unable to generate credentials: %w", err) + return span.Error(fmt.Errorf("unable to generate credentials: %w", err)) } cs.Status.Created = time.Now() @@ -141,7 +150,7 @@ func (p *Porter) GenerateCredentials(ctx context.Context, opts CredentialOptions err = p.Credentials.UpsertCredentialSet(ctx, cs) if err != nil { - return fmt.Errorf("unable to save credentials: %w", err) + return span.Error(fmt.Errorf("unable to save credentials: %w", err)) } return nil @@ -167,6 +176,9 @@ func (o *CredentialEditOptions) Validate(args []string) error { // EditCredential edits the credentials of the provided name. func (p *Porter) EditCredential(ctx context.Context, opts CredentialEditOptions) error { + ctx, span := tracing.StartSpan(ctx) + defer span.EndSpan() + credSet, err := p.Credentials.GetCredentialSet(ctx, opts.Namespace, opts.Name) if err != nil { return err @@ -175,29 +187,29 @@ func (p *Porter) EditCredential(ctx context.Context, opts CredentialEditOptions) // TODO(carolynvs): support editing in yaml, json or toml contents, err := encoding.MarshalYaml(credSet) if err != nil { - return fmt.Errorf("unable to load credentials: %w", err) + return span.Error(fmt.Errorf("unable to load credentials: %w", err)) } editor := editor.New(p.Context, fmt.Sprintf("porter-%s.yaml", credSet.Name), contents) output, err := editor.Run(ctx) if err != nil { - return fmt.Errorf("unable to open editor to edit credentials: %w", err) + return span.Error(fmt.Errorf("unable to open editor to edit credentials: %w", err)) } err = encoding.UnmarshalYaml(output, &credSet) if err != nil { - return fmt.Errorf("unable to process credentials: %w", err) + return span.Error(fmt.Errorf("unable to process credentials: %w", err)) } err = p.Credentials.Validate(ctx, credSet) if err != nil { - return fmt.Errorf("credentials are invalid: %w", err) + return span.Error(fmt.Errorf("credentials are invalid: %w", err)) } credSet.Status.Modified = time.Now() err = p.Credentials.UpdateCredentialSet(ctx, credSet) if err != nil { - return fmt.Errorf("unable to save credentials: %w", err) + return span.Error(fmt.Errorf("unable to save credentials: %w", err)) } return nil @@ -212,6 +224,9 @@ type DisplayCredentialSet struct { // ShowCredential shows the credential set corresponding to the provided name, using // the provided printer.PrintOptions for display. func (p *Porter) ShowCredential(ctx context.Context, opts CredentialShowOptions) error { + ctx, span := tracing.StartSpan(ctx) + defer span.EndSpan() + cs, err := p.Credentials.GetCredentialSet(ctx, opts.Namespace, opts.Name) if err != nil { return err @@ -228,6 +243,8 @@ func (p *Porter) ShowCredential(ctx context.Context, opts CredentialShowOptions) if err != nil { return err } + + // Note that we are not using span.Info because the command's output must go to standard out fmt.Fprintln(p.Out, string(result)) return nil case printer.FormatPlaintext: @@ -257,6 +274,7 @@ func (p *Porter) ShowCredential(ctx context.Context, opts CredentialShowOptions) table.SetAutoFormatHeaders(false) // First, print the CredentialSet metadata + // Note that we are not using span.Info because the command's output must go to standard out fmt.Fprintf(p.Out, "Name: %s\n", credSet.Name) fmt.Fprintf(p.Out, "Namespace: %s\n", credSet.Namespace) fmt.Fprintf(p.Out, "Created: %s\n", tp.Format(credSet.Status.Created)) @@ -280,7 +298,7 @@ func (p *Porter) ShowCredential(ctx context.Context, opts CredentialShowOptions) table.Render() return nil default: - return fmt.Errorf("invalid format: %s", opts.Format) + return span.Error(fmt.Errorf("invalid format: %s", opts.Format)) } } @@ -293,15 +311,19 @@ type CredentialDeleteOptions struct { // DeleteCredential deletes the credential set corresponding to the provided // names. func (p *Porter) DeleteCredential(ctx context.Context, opts CredentialDeleteOptions) error { + ctx, span := tracing.StartSpan(ctx, + attribute.String("namespace", opts.Namespace), + attribute.String("name", opts.Name), + ) + defer span.EndSpan() + err := p.Credentials.RemoveCredentialSet(ctx, opts.Namespace, opts.Name) if errors.Is(err, storage.ErrNotFound{}) { - if p.Debug { - fmt.Fprintln(p.Err, err) - } + span.Debug("nothing to remove, credential already does not exist") return nil } if err != nil { - return fmt.Errorf("unable to delete credential set: %w", err) + return span.Error(fmt.Errorf("unable to delete credential set: %w", err)) } return nil @@ -328,29 +350,23 @@ func validateCredentialName(args []string) error { } func (p *Porter) CredentialsApply(ctx context.Context, o ApplyOptions) error { - if p.Debug { - fmt.Fprintf(p.Err, "Reading input file %s...\n", o.File) - } + ctx, span := tracing.StartSpan(ctx) + defer span.EndSpan() + span.Debugf("Reading input file %s...\n", o.File) namespace, err := p.getNamespaceFromFile(o) if err != nil { - return err - } - - if p.Debug { - // ignoring any error here, printing debug info isn't critical - contents, _ := p.FileSystem.ReadFile(o.File) - fmt.Fprintf(p.Err, "Input file contents:\n%s\n", contents) + return span.Error(err) } var creds storage.CredentialSet err = encoding.UnmarshalFile(p.FileSystem, o.File, &creds) if err != nil { - return fmt.Errorf("could not load %s as a credential set: %w", o.File, err) + return span.Error(fmt.Errorf("could not load %s as a credential set: %w", o.File, err)) } if err = creds.Validate(); err != nil { - return fmt.Errorf("invalid credential set: %w", err) + return span.Error(fmt.Errorf("invalid credential set: %w", err)) } creds.Namespace = namespace @@ -358,7 +374,7 @@ func (p *Porter) CredentialsApply(ctx context.Context, o ApplyOptions) error { err = p.Credentials.Validate(ctx, creds) if err != nil { - return fmt.Errorf("credential set is invalid: %w", err) + return span.Error(fmt.Errorf("credential set is invalid: %w", err)) } err = p.Credentials.UpsertCredentialSet(ctx, creds) @@ -366,7 +382,7 @@ func (p *Porter) CredentialsApply(ctx context.Context, o ApplyOptions) error { return err } - fmt.Fprintf(p.Err, "Applied %s credential set\n", creds) + span.Infof("Applied %s credential set", creds) return nil } @@ -411,7 +427,11 @@ func (o *CredentialCreateOptions) Validate(args []string) error { return nil } -func (p *Porter) CreateCredential(opts CredentialCreateOptions) error { +func (p *Porter) CreateCredential(ctx context.Context, opts CredentialCreateOptions) error { + //lint:ignore SA4006 ignore unused ctx for now + ctx, span := tracing.StartSpan(ctx) + defer span.EndSpan() + if opts.OutputType == "" { opts.OutputType = strings.Trim(filepath.Ext(opts.FileName), ".") } @@ -427,6 +447,8 @@ func (p *Porter) CreateCredential(opts CredentialCreateOptions) error { if err != nil { return err } + + // Note that we are not using span.Info because this must be printed to stdout fmt.Fprintln(p.Out, string(credentialSet)) return nil @@ -435,24 +457,28 @@ func (p *Porter) CreateCredential(opts CredentialCreateOptions) error { if err != nil { return err } + + // Note that we are not using span.Info because this must be printed to stdout fmt.Fprintln(p.Out, string(credentialSet)) return nil default: - return newUnsupportedFormatError(opts.OutputType) + return span.Error(newUnsupportedFormatError(opts.OutputType)) } } - fmt.Fprintln(p.Err, "creating porter credential set in the current directory") + span.Info("creating porter credential set in the current directory") switch opts.OutputType { case "json": - return p.CopyTemplate(p.Templates.GetCredentialSetJSON, opts.FileName) + err := p.CopyTemplate(p.Templates.GetCredentialSetJSON, opts.FileName) + return span.Error(err) case "yaml", "yml": - return p.CopyTemplate(p.Templates.GetCredentialSetYAML, opts.FileName) + err := p.CopyTemplate(p.Templates.GetCredentialSetYAML, opts.FileName) + return span.Error(err) default: - return newUnsupportedFormatError(opts.OutputType) + return span.Error(newUnsupportedFormatError(opts.OutputType)) } } diff --git a/pkg/porter/credentials_test.go b/pkg/porter/credentials_test.go index 7955845ae..a10aed68f 100644 --- a/pkg/porter/credentials_test.go +++ b/pkg/porter/credentials_test.go @@ -506,11 +506,12 @@ func TestCredentialsCreate(t *testing.T) { for _, tc := range testcases { t.Run(tc.name, func(t *testing.T) { + ctx := context.Background() p := NewTestPorter(t) defer p.Close() opts := CredentialCreateOptions{FileName: tc.fileName, OutputType: tc.outputType} - err := p.CreateCredential(opts) + err := p.CreateCredential(ctx, opts) if tc.wantErr == "" { require.NoError(t, err, "no error should have existed") return diff --git a/pkg/porter/dependencies.go b/pkg/porter/dependencies.go index 75154c34e..047902491 100644 --- a/pkg/porter/dependencies.go +++ b/pkg/porter/dependencies.go @@ -13,6 +13,7 @@ import ( "get.porter.sh/porter/pkg/manifest" "get.porter.sh/porter/pkg/runtime" "get.porter.sh/porter/pkg/storage" + "get.porter.sh/porter/pkg/tracing" "github.com/hashicorp/go-multierror" ) @@ -60,6 +61,9 @@ type queuedDependency struct { } func (e *dependencyExecutioner) Prepare(ctx context.Context) error { + ctx, span := tracing.StartSpan(ctx) + defer span.EndSpan() + parentActionArgs, err := e.porter.BuildActionArgs(ctx, e.parentInstallation, e.parentAction) if err != nil { return err @@ -82,11 +86,14 @@ func (e *dependencyExecutioner) Prepare(ctx context.Context) error { } func (e *dependencyExecutioner) Execute(ctx context.Context) error { + ctx, span := tracing.StartSpan(ctx) + defer span.EndSpan() + if e.deps == nil { - return errors.New("Prepare must be called before Execute") + return span.Error(errors.New("Prepare must be called before Execute")) } - // executeDependency the requested action against all of the dependencies + // executeDependency the requested action against all the dependencies for _, dep := range e.deps { err := e.executeDependency(ctx, dep) if err != nil { @@ -121,18 +128,21 @@ func (e *dependencyExecutioner) PrepareRootActionArguments(ctx context.Context) } func (e *dependencyExecutioner) identifyDependencies(ctx context.Context) error { + ctx, span := tracing.StartSpan(ctx) + defer span.EndSpan() + // Load parent CNAB bundle definition var bun cnab.ExtendedBundle if e.parentOpts.CNABFile != "" { bundle, err := e.CNAB.LoadBundle(e.parentOpts.CNABFile) if err != nil { - return err + return span.Error(err) } bun = bundle } else if e.parentOpts.Reference != "" { - cachedBundle, err := e.Resolver.Resolve(e.parentOpts.BundlePullOptions) + cachedBundle, err := e.Resolver.Resolve(ctx, e.parentOpts.BundlePullOptions) if err != nil { - return fmt.Errorf("could not resolve bundle: %w", err) + return span.Error(fmt.Errorf("could not resolve bundle: %w", err)) } bun = cachedBundle.Definition @@ -145,20 +155,18 @@ func (e *dependencyExecutioner) identifyDependencies(ctx context.Context) error bun = cnab.NewBundle(c.Bundle) } else { // If we hit here, there is a bug somewhere - return errors.New("identifyDependencies failed to load the bundle because no bundle was specified. Please report this bug to https://github.com/getporter/porter/issues/new/choose") + return span.Error(errors.New("identifyDependencies failed to load the bundle because no bundle was specified. Please report this bug to https://github.com/getporter/porter/issues/new/choose")) } solver := &cnab.DependencySolver{} locks, err := solver.ResolveDependencies(bun) if err != nil { - return err + return span.Error(err) } e.deps = make([]*queuedDependency, len(locks)) for i, lock := range locks { - if e.Debug { - fmt.Fprintf(e.Out, "Resolved dependency %s to %s\n", lock.Alias, lock.Reference) - } + span.Debugf("Resolved dependency %s to %s", lock.Alias, lock.Reference) e.deps[i] = &queuedDependency{ DependencyLock: lock, } @@ -168,6 +176,9 @@ func (e *dependencyExecutioner) identifyDependencies(ctx context.Context) error } func (e *dependencyExecutioner) prepareDependency(ctx context.Context, dep *queuedDependency) error { + ctx, span := tracing.StartSpan(ctx) + defer span.EndSpan() + // Pull the dependency var err error pullOpts := BundlePullOptions{ @@ -176,23 +187,23 @@ func (e *dependencyExecutioner) prepareDependency(ctx context.Context, dep *queu Force: e.parentOpts.Force, } if err := pullOpts.Validate(); err != nil { - return fmt.Errorf("error preparing dependency %s: %w", dep.Alias, err) + return span.Error(fmt.Errorf("error preparing dependency %s: %w", dep.Alias, err)) } - cachedDep, err := e.Resolver.Resolve(pullOpts) + cachedDep, err := e.Resolver.Resolve(ctx, pullOpts) if err != nil { - return fmt.Errorf("error pulling dependency %s: %w", dep.Alias, err) + return span.Error(fmt.Errorf("error pulling dependency %s: %w", dep.Alias, err)) } dep.BundleReference = cachedDep.BundleReference err = cachedDep.Definition.Validate() if err != nil { - return fmt.Errorf("invalid bundle %s: %w", dep.Alias, err) + return span.Error(fmt.Errorf("invalid bundle %s: %w", dep.Alias, err)) } // Cache the bundle.json for later dep.cnabFileContents, err = e.FileSystem.ReadFile(cachedDep.BundlePath) if err != nil { - return fmt.Errorf("error reading %s: %w", cachedDep.BundlePath, err) + return span.Error(fmt.Errorf("error reading %s: %w", cachedDep.BundlePath, err)) } // Make a lookup of which parameters are defined in the dependent bundle @@ -262,6 +273,9 @@ func (e *dependencyExecutioner) executeDependency(ctx context.Context, dep *queu // even the root bundle uses the execution engine here. This would set up how // we want dependencies and mixins as bundles to work in the future. + ctx, span := tracing.StartSpan(ctx) + defer span.EndSpan() + depName := depsv1.BuildPrerequisiteInstallationName(e.parentOpts.Name, dep.Alias) depInstallation, err := e.Installations.GetInstallation(ctx, e.parentOpts.Namespace, depName) if err != nil { @@ -280,7 +294,7 @@ func (e *dependencyExecutioner) executeDependency(ctx context.Context, dep *queu resolvedParameters, err := e.porter.resolveParameters(ctx, depInstallation, dep.BundleReference.Definition, e.parentArgs.Action, dep.Parameters) if err != nil { - return fmt.Errorf("error resolving parameters for dependency %s: %w", dep.Alias, err) + return span.Error(fmt.Errorf("error resolving parameters for dependency %s: %w", dep.Alias, err)) } depArgs := cnabprovider.ActionArguments{ @@ -301,23 +315,23 @@ func (e *dependencyExecutioner) executeDependency(ctx context.Context, dep *queu } var executeErrs error - fmt.Fprintf(e.Out, "Executing dependency %s...\n", dep.Alias) + span.Infof("Executing dependency %s...", dep.Alias) err = e.CNAB.Execute(ctx, depArgs) if err != nil { executeErrs = multierror.Append(executeErrs, fmt.Errorf("error executing dependency %s: %w", dep.Alias, err)) // Handle errors when/if the action is uninstall // If uninstallOpts is an empty struct, executeErrs will pass through - executeErrs = uninstallOpts.handleUninstallErrs(e.Out, executeErrs) + executeErrs = uninstallOpts.handleUninstallErrs(e.Err, executeErrs) if executeErrs != nil { - return executeErrs + return span.Error(executeErrs) } } // If uninstallOpts is an empty struct (i.e., action not Uninstall), this // will resolve to false and thus be a no-op if uninstallOpts.shouldDelete() { - fmt.Fprintf(e.Out, installationDeleteTmpl, depArgs.Installation) + span.Infof(installationDeleteTmpl, depArgs.Installation) return e.Installations.RemoveInstallation(ctx, depArgs.Installation.Namespace, depArgs.Installation.Name) } return nil diff --git a/pkg/porter/examples/pull_example_test.go b/pkg/porter/examples/pull_example_test.go index bcc858a96..62acd60e3 100644 --- a/pkg/porter/examples/pull_example_test.go +++ b/pkg/porter/examples/pull_example_test.go @@ -1,6 +1,7 @@ package examples_test import ( + "context" "fmt" "log" @@ -8,6 +9,7 @@ import ( ) func ExamplePorter_pullBundle() { + ctx := context.Background() // Create an instance of the Porter application p := porter.New() @@ -18,7 +20,7 @@ func ExamplePorter_pullBundle() { // Pull a bundle to Porter's cache, ~/.porter/cache // This isn't exposed as a command in Porter's CLI - cachedBundle, err := p.PullBundle(pullOpts) + cachedBundle, err := p.PullBundle(ctx, pullOpts) if err != nil { log.Fatal(err) } diff --git a/pkg/porter/lifecycle.go b/pkg/porter/lifecycle.go index 54fbd2ad7..38f1964fc 100644 --- a/pkg/porter/lifecycle.go +++ b/pkg/porter/lifecycle.go @@ -94,7 +94,7 @@ func (p *Porter) resolveBundleReference(ctx context.Context, opts *BundleActionO useReference := func(ref cnab.OCIReference) error { pullOpts := *opts // make a copy just to do the pull pullOpts.Reference = ref.String() - cachedBundle, err := p.prepullBundleByReference(&pullOpts) + cachedBundle, err := p.prepullBundleByReference(ctx, &pullOpts) if err != nil { return err } @@ -218,12 +218,12 @@ func (p *Porter) BuildActionArgs(ctx context.Context, installation storage.Insta // prepullBundleByReference handles calling the bundle pull operation and updating // the shared options like name and bundle file path. This is used by install, upgrade // and uninstall -func (p *Porter) prepullBundleByReference(opts *BundleActionOptions) (cache.CachedBundle, error) { +func (p *Porter) prepullBundleByReference(ctx context.Context, opts *BundleActionOptions) (cache.CachedBundle, error) { if opts.Reference == "" { return cache.CachedBundle{}, nil } - cachedBundle, err := p.PullBundle(opts.BundlePullOptions) + cachedBundle, err := p.PullBundle(ctx, opts.BundlePullOptions) if err != nil { return cache.CachedBundle{}, err } diff --git a/pkg/porter/lifecycle_test.go b/pkg/porter/lifecycle_test.go index 8610d348d..107234022 100644 --- a/pkg/porter/lifecycle_test.go +++ b/pkg/porter/lifecycle_test.go @@ -34,13 +34,15 @@ func TestInstallFromTagIgnoresCurrentBundle(t *testing.T) { } func TestPorter_BuildActionArgs(t *testing.T) { + ctx := context.Background() + t.Run("no bundle set", func(t *testing.T) { p := NewTestPorter(t) defer p.Close() opts := NewInstallOptions() opts.Name = "mybuns" - err := opts.Validate(context.Background(), nil, p.Porter) + err := opts.Validate(ctx, nil, p.Porter) require.Error(t, err, "Validate should fail") assert.Contains(t, err.Error(), "No bundle specified") }) @@ -54,9 +56,9 @@ func TestPorter_BuildActionArgs(t *testing.T) { p.TestConfig.TestContext.AddTestFile("testdata/porter.yaml", "porter.yaml") p.TestConfig.TestContext.AddTestFile("testdata/bundle.json", ".cnab/bundle.json") - err := opts.Validate(context.Background(), nil, p.Porter) + err := opts.Validate(ctx, nil, p.Porter) require.NoError(t, err, "Validate failed") - args, err := p.BuildActionArgs(context.TODO(), storage.Installation{}, opts) + args, err := p.BuildActionArgs(ctx, storage.Installation{}, opts) require.NoError(t, err, "BuildActionArgs failed") assert.NotEmpty(t, args.BundleReference.Definition) @@ -69,9 +71,9 @@ func TestPorter_BuildActionArgs(t *testing.T) { opts.CNABFile = "/bundle.json" p.TestConfig.TestContext.AddTestFile("testdata/bundle.json", "/bundle.json") - err := opts.Validate(context.Background(), nil, p.Porter) + err := opts.Validate(ctx, nil, p.Porter) require.NoError(t, err, "Validate failed") - args, err := p.BuildActionArgs(context.TODO(), storage.Installation{}, opts) + args, err := p.BuildActionArgs(ctx, storage.Installation{}, opts) require.NoError(t, err, "BuildActionArgs failed") assert.NotEmpty(t, args.BundleReference.Definition, "BundlePath was not populated correctly") @@ -106,10 +108,10 @@ func TestPorter_BuildActionArgs(t *testing.T) { p.TestParameters.AddSecret("PARAM2_SECRET", "VALUE2") p.TestParameters.AddTestParameters("testdata/paramset2.json") - err := opts.Validate(context.Background(), nil, p.Porter) + err := opts.Validate(ctx, nil, p.Porter) require.NoError(t, err, "Validate failed") existingInstall := storage.Installation{Name: opts.Name} - args, err := p.BuildActionArgs(context.TODO(), existingInstall, opts) + args, err := p.BuildActionArgs(ctx, existingInstall, opts) require.NoError(t, err, "BuildActionArgs failed") expectedParams := map[string]interface{}{ diff --git a/pkg/porter/mixins.go b/pkg/porter/mixins.go index 20daed594..5ed6a6425 100644 --- a/pkg/porter/mixins.go +++ b/pkg/porter/mixins.go @@ -92,8 +92,8 @@ func (p *Porter) InstallMixin(ctx context.Context, opts mixin.InstallOptions) er return nil } -func (p *Porter) UninstallMixin(opts pkgmgmt.UninstallOptions) error { - err := p.Mixins.Uninstall(opts) +func (p *Porter) UninstallMixin(ctx context.Context, opts pkgmgmt.UninstallOptions) error { + err := p.Mixins.Uninstall(ctx, opts) if err != nil { return err } @@ -103,10 +103,10 @@ func (p *Porter) UninstallMixin(opts pkgmgmt.UninstallOptions) error { return nil } -func (p *Porter) GenerateMixinFeed(opts feed.GenerateOptions) error { +func (p *Porter) GenerateMixinFeed(ctx context.Context, opts feed.GenerateOptions) error { f := feed.NewMixinFeed(p.Context) - err := f.Generate(opts) + err := f.Generate(ctx, opts) if err != nil { return err } diff --git a/pkg/porter/mixins_test.go b/pkg/porter/mixins_test.go index ade73d75e..e9876a8fd 100644 --- a/pkg/porter/mixins_test.go +++ b/pkg/porter/mixins_test.go @@ -48,6 +48,7 @@ func TestPorter_InstallMixin(t *testing.T) { } func TestPorter_UninstallMixin(t *testing.T) { + ctx := context.Background() p := NewTestPorter(t) defer p.Close() @@ -55,7 +56,7 @@ func TestPorter_UninstallMixin(t *testing.T) { err := opts.Validate([]string{"exec"}) require.NoError(t, err, "Validate failed") - err = p.UninstallMixin(opts) + err = p.UninstallMixin(ctx, opts) require.NoError(t, err, "UninstallMixin failed") wantOutput := "Uninstalled exec mixin" diff --git a/pkg/porter/parameters.go b/pkg/porter/parameters.go index ba3b7af5a..b5eb25691 100644 --- a/pkg/porter/parameters.go +++ b/pkg/porter/parameters.go @@ -19,6 +19,7 @@ import ( "get.porter.sh/porter/pkg/printer" "get.porter.sh/porter/pkg/secrets" "get.porter.sh/porter/pkg/storage" + "get.porter.sh/porter/pkg/tracing" dtprinter "github.com/carolynvs/datetime-printer" "github.com/cnabio/cnab-go/bundle" "github.com/cnabio/cnab-go/bundle/definition" @@ -305,15 +306,16 @@ type ParameterDeleteOptions struct { // DeleteParameter deletes the parameter set corresponding to the provided // names. func (p *Porter) DeleteParameter(ctx context.Context, opts ParameterDeleteOptions) error { + ctx, span := tracing.StartSpan(ctx) + defer span.EndSpan() + err := p.Parameters.RemoveParameterSet(ctx, opts.Namespace, opts.Name) if errors.Is(err, storage.ErrNotFound{}) { - if p.Debug { - fmt.Fprintln(p.Err, err) - } + span.Debug("Cannot remove parameter set because it already doesn't exist") return nil } if err != nil { - return fmt.Errorf("unable to delete parameter set: %w", err) + return span.Error(fmt.Errorf("unable to delete parameter set: %w", err)) } return nil @@ -497,29 +499,23 @@ func (p *Porter) printDisplayValuesTable(values []DisplayValue) error { } func (p *Porter) ParametersApply(ctx context.Context, o ApplyOptions) error { - if p.Debug { - fmt.Fprintf(p.Err, "Reading input file %s...\n", o.File) - } + ctx, span := tracing.StartSpan(ctx) + defer span.EndSpan() + span.Debugf("Reading input file %s...", o.File) namespace, err := p.getNamespaceFromFile(o) if err != nil { - return err - } - - if p.Debug { - // ignoring any error here, printing debug info isn't critical - contents, _ := p.FileSystem.ReadFile(o.File) - fmt.Fprintf(p.Err, "Input file contents:\n%s\n", contents) + return span.Error(err) } var params storage.ParameterSet err = encoding.UnmarshalFile(p.FileSystem, o.File, ¶ms) if err != nil { - return fmt.Errorf("could not load %s as a parameter set: %w", o.File, err) + return span.Error(fmt.Errorf("could not load %s as a parameter set: %w", o.File, err)) } if err = params.Validate(); err != nil { - return fmt.Errorf("invalid parameter set: %w", err) + return span.Error(fmt.Errorf("invalid parameter set: %w", err)) } params.Namespace = namespace @@ -527,7 +523,7 @@ func (p *Porter) ParametersApply(ctx context.Context, o ApplyOptions) error { err = p.Parameters.Validate(ctx, params) if err != nil { - return fmt.Errorf("parameter set is invalid: %w", err) + return span.Error(fmt.Errorf("parameter set is invalid: %w", err)) } err = p.Parameters.UpsertParameterSet(ctx, params) @@ -535,7 +531,7 @@ func (p *Porter) ParametersApply(ctx context.Context, o ApplyOptions) error { return err } - fmt.Fprintf(p.Err, "Applied %s parameter set\n", params) + span.Infof("Applied %s parameter set", params) return nil } @@ -619,27 +615,23 @@ func (p *Porter) getUnconvertedValueFromRaw(b cnab.ExtendedBundle, def *definiti } func (p *Porter) resolveParameterSources(ctx context.Context, bun cnab.ExtendedBundle, installation storage.Installation) (secrets.Set, error) { + ctx, span := tracing.StartSpan(ctx) + defer span.EndSpan() + if !bun.HasParameterSources() { - if p.Debug { - fmt.Fprintln(p.Err, "No parameter sources defined, skipping") - } + span.Debug("No parameter sources defined, skipping") return nil, nil } - if p.Debug { - fmt.Fprintln(p.Err, "Resolving parameter sources...") - } - + span.Debug("Resolving parameter sources...") parameterSources, err := bun.ReadParameterSources() if err != nil { - return nil, err + return nil, span.Error(err) } values := secrets.Set{} for parameterName, parameterSource := range parameterSources { - if p.Debug { - fmt.Fprintln(p.Err, "Resolving parameter source", parameterName) - } + span.Debugf("Resolving parameter source %s", parameterName) for _, rawSource := range parameterSource.ListSourcesByPriority() { var installationName string var outputName string @@ -657,32 +649,29 @@ func (p *Porter) resolveParameterSources(ctx context.Context, bun cnab.ExtendedB if err != nil { // When we can't find the output, skip it and let the parameter be set another way if errors.Is(err, storage.ErrNotFound{}) { - if p.Debug { - fmt.Fprintf(p.Err, "No previous output found for %s from %s/%s\n", outputName, installation.Namespace, installationName) - } + span.Debugf("No previous output found for %s from %s/%s", outputName, installation.Namespace, installationName) continue } // Otherwise, something else has happened, perhaps bad data or connectivity problems, we can't ignore it - return nil, fmt.Errorf("could not set parameter %s from output %s of %s: %w", parameterName, outputName, installation, err) + return nil, span.Error(fmt.Errorf("could not set parameter %s from output %s of %s: %w", parameterName, outputName, installation, err)) } if output.Key != "" { - resolved, err := p.Sanitizer.RestoreOutput(ctx, output) if err != nil { - return nil, fmt.Errorf("could not resolve %s's output %s: %w", installation, outputName, err) + return nil, span.Error(fmt.Errorf("could not resolve %s's output %s: %w", installation, outputName, err)) } output = resolved } param, ok := bun.Parameters[parameterName] if !ok { - return nil, fmt.Errorf("resolveParameterSources: %s not defined in bundle", parameterName) + return nil, span.Error(fmt.Errorf("resolveParameterSources: %s not defined in bundle", parameterName)) } def, ok := bun.Definitions[param.Definition] if !ok { - return nil, fmt.Errorf("definition %s not defined in bundle", param.Definition) + return nil, span.Error(fmt.Errorf("definition %s not defined in bundle", param.Definition)) } if bun.IsFileType(def) { @@ -691,9 +680,7 @@ func (p *Porter) resolveParameterSources(ctx context.Context, bun cnab.ExtendedB values[parameterName] = string(output.Value) } - if p.Debug { - fmt.Fprintf(p.Out, "Injected installation %s output %s as parameter %s\n", installation, outputName, parameterName) - } + span.Debugf("Injected installation %s output %s as parameter %s", installation, outputName, parameterName) } } diff --git a/pkg/porter/plugins.go b/pkg/porter/plugins.go index 83895a8e7..5a8d5261d 100644 --- a/pkg/porter/plugins.go +++ b/pkg/porter/plugins.go @@ -167,8 +167,8 @@ func (p *Porter) InstallPlugin(ctx context.Context, opts plugins.InstallOptions) return nil } -func (p *Porter) UninstallPlugin(opts pkgmgmt.UninstallOptions) error { - err := p.Plugins.Uninstall(opts) +func (p *Porter) UninstallPlugin(ctx context.Context, opts pkgmgmt.UninstallOptions) error { + err := p.Plugins.Uninstall(ctx, opts) if err != nil { return err } diff --git a/pkg/porter/plugins_test.go b/pkg/porter/plugins_test.go index 96e8f6c05..64441a108 100644 --- a/pkg/porter/plugins_test.go +++ b/pkg/porter/plugins_test.go @@ -238,6 +238,7 @@ func TestPorter_InstallPlugin(t *testing.T) { } func TestPorter_UninstallPlugin(t *testing.T) { + ctx := context.Background() p := NewTestPorter(t) defer p.Close() @@ -245,7 +246,7 @@ func TestPorter_UninstallPlugin(t *testing.T) { err := opts.Validate([]string{"plugin1"}) require.NoError(t, err, "Validate failed") - err = p.UninstallPlugin(opts) + err = p.UninstallPlugin(ctx, opts) require.NoError(t, err, "UninstallPlugin failed") wantOutput := "Uninstalled plugin1 plugin" diff --git a/pkg/porter/porter.go b/pkg/porter/porter.go index 84d76b335..621479d99 100644 --- a/pkg/porter/porter.go +++ b/pkg/porter/porter.go @@ -3,7 +3,6 @@ package porter import ( "context" "errors" - "fmt" "strings" "get.porter.sh/porter/pkg/build" @@ -100,10 +99,6 @@ func (p *Porter) Connect(ctx context.Context) error { // Close releases resources used by Porter before terminating the application. func (p *Porter) Close() error { - if p.Debug { - fmt.Fprintln(p.Err, "Closing plugins") - } - // Shutdown our plugins var bigErr *multierror.Error diff --git a/pkg/porter/porter_integration_test.go b/pkg/porter/porter_integration_test.go index 2d2f20d2d..a880da8dd 100644 --- a/pkg/porter/porter_integration_test.go +++ b/pkg/porter/porter_integration_test.go @@ -1,5 +1,4 @@ //go:build integration -// +build integration package porter @@ -15,7 +14,7 @@ import ( func TestPorter_FixPermissions(t *testing.T) { p := NewTestPorter(t) - p.SetupIntegrationTest() + ctx := p.SetupIntegrationTest() defer p.Close() home, _ := p.GetHomeDir() @@ -31,7 +30,7 @@ func TestPorter_FixPermissions(t *testing.T) { require.NoError(t, os.MkdirAll(dir, pkg.FileModeDirectory)) require.NoError(t, os.WriteFile(tc, []byte(""), 0750)) - err := p.FixPermissions() + err := p.FixPermissions(ctx) require.NoError(t, err) // Check that all files in the directory have the correct permissions @@ -42,7 +41,7 @@ func TestPorter_FixPermissions(t *testing.T) { func TestPorter_FixPermissions_NoConfigFile(t *testing.T) { p := NewTestPorter(t) - p.SetupIntegrationTest() + ctx := p.SetupIntegrationTest() defer p.Close() // Remember the original permissions on the current working directory @@ -51,7 +50,7 @@ func TestPorter_FixPermissions_NoConfigFile(t *testing.T) { require.NoError(t, err, "stat on the current working directory failed") wantMode := wdInfo.Mode() - err = p.FixPermissions() + err = p.FixPermissions(ctx) require.NoError(t, err) // Check that the current working directory didn't have its permissions changed diff --git a/pkg/porter/publish.go b/pkg/porter/publish.go index 037ada394..7bcfebb01 100644 --- a/pkg/porter/publish.go +++ b/pkg/porter/publish.go @@ -211,7 +211,7 @@ func (p *Porter) publishFromArchive(ctx context.Context, opts PublishOptions) er } defer p.FileSystem.RemoveAll(tmpDir) - bundleRef, err := p.extractBundle(tmpDir, source) + bundleRef, err := p.extractBundle(ctx, tmpDir, source) if err != nil { return err } @@ -258,30 +258,31 @@ func (p *Porter) publishFromArchive(ctx context.Context, opts PublishOptions) er } // extractBundle extracts a bundle using the provided opts and returns the extracted bundle -func (p *Porter) extractBundle(tmpDir, source string) (cnab.BundleReference, error) { - if p.Debug { - fmt.Fprintf(p.Err, "Extracting bundle from archive %s...\n", source) - } +func (p *Porter) extractBundle(ctx context.Context, tmpDir, source string) (cnab.BundleReference, error) { + //lint:ignore SA4006 ignore unused ctx for now + ctx, span := tracing.StartSpan(ctx) + defer span.EndSpan() + span.Debugf("Extracting bundle from archive %s...", source) l := loader.NewLoader() imp := packager.NewImporter(source, tmpDir, l) err := imp.Import() if err != nil { - return cnab.BundleReference{}, fmt.Errorf("failed to extract bundle from archive %s: %w", source, err) + return cnab.BundleReference{}, span.Error(fmt.Errorf("failed to extract bundle from archive %s: %w", source, err)) } bun, err := l.Load(filepath.Join(tmpDir, strings.TrimSuffix(filepath.Base(source), ".tgz"), "bundle.json")) if err != nil { - return cnab.BundleReference{}, fmt.Errorf("failed to load bundle from archive %s: %w", source, err) + return cnab.BundleReference{}, span.Error(fmt.Errorf("failed to load bundle from archive %s: %w", source, err)) } data, err := p.FileSystem.ReadFile(filepath.Join(tmpDir, strings.TrimSuffix(filepath.Base(source), ".tgz"), "relocation-mapping.json")) if err != nil { - return cnab.BundleReference{}, fmt.Errorf("failed to load relocation-mapping.json from archive %s: %w", source, err) + return cnab.BundleReference{}, span.Error(fmt.Errorf("failed to load relocation-mapping.json from archive %s: %w", source, err)) } var reloMap relocation.ImageRelocationMap err = json.Unmarshal(data, &reloMap) if err != nil { - return cnab.BundleReference{}, fmt.Errorf("failed to parse relocation-mapping.json from archive %s: %w", source, err) + return cnab.BundleReference{}, span.Error(fmt.Errorf("failed to parse relocation-mapping.json from archive %s: %w", source, err)) } return cnab.BundleReference{Definition: cnab.ExtendedBundle{Bundle: *bun}, RelocationMap: reloMap}, nil diff --git a/pkg/porter/pull.go b/pkg/porter/pull.go index d763a5165..12785fd1d 100644 --- a/pkg/porter/pull.go +++ b/pkg/porter/pull.go @@ -1,6 +1,7 @@ package porter import ( + "context" "fmt" "get.porter.sh/porter/pkg/cache" @@ -40,10 +41,10 @@ func (b *BundlePullOptions) validateReference() error { // PullBundle looks for a given bundle tag in the bundle cache. If it is not found, it is // pulled and stored in the cache. The path to the cached bundle is returned. -func (p *Porter) PullBundle(opts BundlePullOptions) (cache.CachedBundle, error) { +func (p *Porter) PullBundle(ctx context.Context, opts BundlePullOptions) (cache.CachedBundle, error) { resolver := BundleResolver{ Cache: p.Cache, Registry: p.Registry, } - return resolver.Resolve(opts) + return resolver.Resolve(ctx, opts) } diff --git a/pkg/porter/reconcile.go b/pkg/porter/reconcile.go index 28d9b40a4..1ae65d782 100644 --- a/pkg/porter/reconcile.go +++ b/pkg/porter/reconcile.go @@ -34,9 +34,7 @@ type ReconcileOptions struct { // to an installation. For uninstall or invoke, you should call those directly. func (p *Porter) ReconcileInstallation(ctx context.Context, opts ReconcileOptions) error { ctx, log := tracing.StartSpan(ctx) - if p.Debug { - fmt.Fprintf(p.Err, "Reconciling %s/%s installation\n", opts.Namespace, opts.Name) - } + log.Debugf("Reconciling %s/%s installation", opts.Namespace, opts.Name) // Get the last run of the installation, if available var lastRun *storage.Run @@ -51,11 +49,11 @@ func (p *Porter) ReconcileInstallation(ctx context.Context, opts ReconcileOption ref, ok, err := opts.Installation.Bundle.GetBundleReference() if err != nil { - return err + return log.Error(err) } if !ok { instYaml, _ := yaml.Marshal(opts.Installation) - return fmt.Errorf("the installation does not define a valid bundle reference.\n%s", instYaml) + return log.Error(fmt.Errorf("the installation does not define a valid bundle reference.\n%s", instYaml)) } // Configure the bundle action that we should execute IF IT'S OUT OF SYNC @@ -107,9 +105,9 @@ func (p *Porter) ReconcileInstallation(ctx context.Context, opts ReconcileOption if inSync { if opts.Force { - fmt.Fprintln(p.Out, "The installation is up-to-date but will be re-applied because --force was specified") + log.Info("The installation is up-to-date but will be re-applied because --force was specified") } else { - fmt.Fprintln(p.Out, "The installation is already up-to-date.") + log.Info("The installation is already up-to-date.") return nil } } @@ -120,7 +118,7 @@ func (p *Porter) ReconcileInstallation(ctx context.Context, opts ReconcileOption } if opts.DryRun { - fmt.Fprintln(p.Out, "Skipping bundle execution because --dry-run was specified") + log.Info("Skipping bundle execution because --dry-run was specified") return nil } diff --git a/pkg/porter/resolver.go b/pkg/porter/resolver.go index 0838c1529..492f63618 100644 --- a/pkg/porter/resolver.go +++ b/pkg/porter/resolver.go @@ -1,10 +1,12 @@ package porter import ( + "context" "fmt" "get.porter.sh/porter/pkg/cache" cnabtooci "get.porter.sh/porter/pkg/cnab/cnab-to-oci" + "get.porter.sh/porter/pkg/tracing" ) type BundleResolver struct { @@ -14,11 +16,13 @@ type BundleResolver struct { // Resolves a bundle from the cache, or pulls it and caches it // Returns the location of the bundle or an error -func (r *BundleResolver) Resolve(opts BundlePullOptions) (cache.CachedBundle, error) { +func (r *BundleResolver) Resolve(ctx context.Context, opts BundlePullOptions) (cache.CachedBundle, error) { + log := tracing.LoggerFromContext(ctx) + if !opts.Force { cachedBundle, ok, err := r.Cache.FindBundle(opts.GetReference()) if err != nil { - return cache.CachedBundle{}, fmt.Errorf("unable to load bundle %s from cache: %w", opts.Reference, err) + return cache.CachedBundle{}, log.Error(fmt.Errorf("unable to load bundle %s from cache: %w", opts.Reference, err)) } // If we found the bundle, return the path to the bundle.json if ok { @@ -26,7 +30,7 @@ func (r *BundleResolver) Resolve(opts BundlePullOptions) (cache.CachedBundle, er } } - bundleRef, err := r.Registry.PullBundle(opts.GetReference(), opts.InsecureRegistry) + bundleRef, err := r.Registry.PullBundle(ctx, opts.GetReference(), opts.InsecureRegistry) if err != nil { return cache.CachedBundle{}, err } diff --git a/pkg/porter/resolver_test.go b/pkg/porter/resolver_test.go index 8e717edc4..4c564e41d 100644 --- a/pkg/porter/resolver_test.go +++ b/pkg/porter/resolver_test.go @@ -1,6 +1,7 @@ package porter import ( + "context" "testing" "get.porter.sh/porter/pkg/cache" @@ -12,6 +13,7 @@ import ( ) func TestBundleResolver_Resolve_ForcePull(t *testing.T) { + ctx := context.Background() tc := config.NewTestConfig(t) testReg := cnabtooci.NewTestRegistry() testCache := cache.NewTestCache(cache.New(tc.Config)) @@ -37,13 +39,14 @@ func TestBundleResolver_Resolve_ForcePull(t *testing.T) { Force: true, } require.NoError(t, opts.Validate()) - resolver.Resolve(opts) + resolver.Resolve(ctx, opts) assert.False(t, cacheSearched, "Force should have skipped the cache") assert.True(t, pulled, "The bundle should have been re-pulled") } func TestBundleResolver_Resolve_CacheHit(t *testing.T) { + ctx := context.Background() tc := config.NewTestConfig(t) testReg := cnabtooci.NewTestRegistry() testCache := cache.NewTestCache(cache.New(tc.Config)) @@ -65,13 +68,14 @@ func TestBundleResolver_Resolve_CacheHit(t *testing.T) { } opts := BundlePullOptions{Reference: "ghcr.io/getporter/examples/porter-hello:v0.2.0"} - resolver.Resolve(opts) + resolver.Resolve(ctx, opts) assert.True(t, cacheSearched, "The cache should be searched when force is not specified") assert.False(t, pulled, "The bundle should NOT be pulled because it was found in the cache") } func TestBundleResolver_Resolve_CacheMiss(t *testing.T) { + ctx := context.Background() tc := config.NewTestConfig(t) testReg := cnabtooci.NewTestRegistry() testCache := cache.NewTestCache(cache.New(tc.Config)) @@ -93,7 +97,7 @@ func TestBundleResolver_Resolve_CacheMiss(t *testing.T) { } opts := BundlePullOptions{Reference: "ghcr.io/getporter/examples/porter-hello:v0.2.0"} - resolver.Resolve(opts) + resolver.Resolve(ctx, opts) assert.True(t, cacheSearched, "The cache should be searched when force is not specified") assert.True(t, pulled, "The bundle should have been pulled because the bundle was not in the cache") diff --git a/pkg/porter/schema.go b/pkg/porter/schema.go index e7ba13dcb..bf7c2a80c 100644 --- a/pkg/porter/schema.go +++ b/pkg/porter/schema.go @@ -7,6 +7,7 @@ import ( "path/filepath" "strings" + "get.porter.sh/porter/pkg/tracing" "github.com/PaesslerAG/jsonpath" ) @@ -29,9 +30,12 @@ func (p *Porter) PrintManifestSchema(ctx context.Context) error { } func (p *Porter) GetManifestSchema(ctx context.Context) (jsonSchema, error) { + ctx, span := tracing.StartSpan(ctx) + defer span.EndSpan() + replacementSchema, err := p.GetReplacementSchema() - if err != nil && p.Debug { - fmt.Fprintln(p.Err, fmt.Errorf("ignoring replacement schema: %w", err)) + if err != nil { + span.Debugf("ignoring replacement schema: %w", err) } if replacementSchema != nil { return replacementSchema, nil @@ -39,20 +43,18 @@ func (p *Porter) GetManifestSchema(ctx context.Context) (jsonSchema, error) { b, err := p.Templates.GetSchema() if err != nil { - return nil, err + return nil, span.Error(err) } manifestSchema := make(jsonSchema) err = json.Unmarshal(b, &manifestSchema) if err != nil { - return nil, fmt.Errorf("could not unmarshal the root porter manifest schema: %w", err) + return nil, span.Error(fmt.Errorf("could not unmarshal the root porter manifest schema: %w", err)) } combinedSchema, err := p.injectMixinSchemas(ctx, manifestSchema) if err != nil { - if p.Debug { - fmt.Fprintln(p.Err, err) - } + span.Warn(err.Error()) // Fallback to the porter schema, without any mixins return manifestSchema, nil } @@ -61,29 +63,32 @@ func (p *Porter) GetManifestSchema(ctx context.Context) (jsonSchema, error) { } func (p *Porter) injectMixinSchemas(ctx context.Context, manifestSchema jsonSchema) (jsonSchema, error) { + ctx, span := tracing.StartSpan(ctx) + defer span.EndSpan() + propertiesSchema, ok := manifestSchema["properties"].(jsonSchema) if !ok { - return nil, fmt.Errorf("root porter manifest schema has invalid properties type, expected map[string]interface{} but got %T", manifestSchema["properties"]) + return nil, span.Error(fmt.Errorf("root porter manifest schema has invalid properties type, expected map[string]interface{} but got %T", manifestSchema["properties"])) } additionalPropertiesSchema, ok := manifestSchema["additionalProperties"].(jsonSchema) if !ok { - return nil, fmt.Errorf("root porter manifest schema has invalid additionalProperties type, expected map[string]interface{} but got %T", manifestSchema["additionalProperties"]) + return nil, span.Error(fmt.Errorf("root porter manifest schema has invalid additionalProperties type, expected map[string]interface{} but got %T", manifestSchema["additionalProperties"])) } mixinSchema, ok := propertiesSchema["mixins"].(jsonSchema) if !ok { - return nil, fmt.Errorf("root porter manifest schema has invalid properties.mixins type, expected map[string]interface{} but got %T", propertiesSchema["mixins"]) + return nil, span.Error(fmt.Errorf("root porter manifest schema has invalid properties.mixins type, expected map[string]interface{} but got %T", propertiesSchema["mixins"])) } mixinItemSchema, ok := mixinSchema["items"].(jsonSchema) if !ok { - return nil, fmt.Errorf("root porter manifest schema has invalid properties.mixins.items type, expected map[string]interface{} but got %T", mixinSchema["items"]) + return nil, span.Error(fmt.Errorf("root porter manifest schema has invalid properties.mixins.items type, expected map[string]interface{} but got %T", mixinSchema["items"])) } mixinEnumSchema, ok := mixinItemSchema["enum"].([]interface{}) if !ok { - return nil, fmt.Errorf("root porter manifest schema has invalid properties.mixins.items.enum type, expected []interface{} but got %T", mixinItemSchema["enum"]) + return nil, span.Error(fmt.Errorf("root porter manifest schema has invalid properties.mixins.items.enum type, expected []interface{} but got %T", mixinItemSchema["enum"])) } coreActions := []string{"install", "upgrade", "uninstall"} // custom actions are defined in json schema as additionalProperties @@ -91,14 +96,14 @@ func (p *Porter) injectMixinSchemas(ctx context.Context, manifestSchema jsonSche for _, action := range coreActions { actionSchema, ok := propertiesSchema[action].(jsonSchema) if !ok { - return nil, fmt.Errorf("root porter manifest schema has invalid properties.%s type, expected map[string]interface{} but got %T", action, propertiesSchema[string(action)]) + return nil, span.Error(fmt.Errorf("root porter manifest schema has invalid properties.%s type, expected map[string]interface{} but got %T", action, propertiesSchema[string(action)])) } actionSchemas[action] = actionSchema } mixins, err := p.Mixins.List() if err != nil { - return nil, err + return nil, span.Error(err) } // If there is an error with any mixin, print a warning and skip the mixin, do not return an error @@ -106,9 +111,7 @@ func (p *Porter) injectMixinSchemas(ctx context.Context, manifestSchema jsonSche mixinSchema, err := p.Mixins.GetSchema(ctx, mixin) if err != nil { // if a mixin can't report its schema, don't include it and keep going - if p.Debug { - fmt.Fprintln(p.Err, fmt.Errorf("could not query mixin %s for its schema: %w", mixin, err)) - } + span.Debugf("could not query mixin %s for its schema: %w", mixin, err) continue } @@ -117,8 +120,8 @@ func (p *Porter) injectMixinSchemas(ctx context.Context, manifestSchema jsonSche mixinSchemaMap := make(jsonSchema) err = json.Unmarshal([]byte(mixinSchema), &mixinSchemaMap) - if err != nil && p.Debug { - fmt.Fprintln(p.Err, fmt.Errorf("could not unmarshal mixin schema for %s, %q: %w", mixin, mixinSchema, err)) + if err != nil { + span.Debugf("could not unmarshal mixin schema for %s, %q: %w", mixin, mixinSchema, err) continue } @@ -129,14 +132,14 @@ func (p *Porter) injectMixinSchemas(ctx context.Context, manifestSchema jsonSche for _, action := range coreActions { actionItemSchema, ok := actionSchemas[action]["items"].(jsonSchema) - if !ok && p.Debug { - fmt.Fprintln(p.Err, fmt.Errorf("root porter manifest schema has invalid properties.%s.items type, expected map[string]interface{} but got %T", action, actionSchemas[string(action)]["items"])) + if !ok { + span.Debugf("root porter manifest schema has invalid properties.%s.items type, expected map[string]interface{} but got %T", action, actionSchemas[string(action)]["items"]) continue } actionAnyOfSchema, ok := actionItemSchema["anyOf"].([]interface{}) - if !ok && p.Debug { - fmt.Fprintln(p.Err, fmt.Errorf("root porter manifest schema has invalid properties.%s.items.anyOf type, expected []interface{} but got %T", action, actionItemSchema["anyOf"])) + if !ok { + span.Debugf("root porter manifest schema has invalid properties.%s.items.anyOf type, expected []interface{} but got %T", action, actionItemSchema["anyOf"]) continue } @@ -150,14 +153,14 @@ func (p *Porter) injectMixinSchemas(ctx context.Context, manifestSchema jsonSche _, err = jsonpath.Get("$.definitions.invokeStep", mixinSchemaMap) if err == nil { actionItemSchema, ok := additionalPropertiesSchema["items"].(jsonSchema) - if !ok && p.Debug { - fmt.Fprintln(p.Err, fmt.Errorf("root porter manifest schema has invalid additionalProperties.items type, expected map[string]interface{} but got %T", additionalPropertiesSchema["items"])) + if !ok { + span.Debugf("root porter manifest schema has invalid additionalProperties.items type, expected map[string]interface{} but got %T", additionalPropertiesSchema["items"]) continue } actionAnyOfSchema, ok := actionItemSchema["anyOf"].([]interface{}) - if !ok && p.Debug { - fmt.Fprintln(p.Err, fmt.Errorf("root porter manifest schema has invalid additionalProperties.items.anyOf type, expected []interface{} but got %T", actionItemSchema["anyOf"])) + if !ok { + span.Debugf("root porter manifest schema has invalid additionalProperties.items.anyOf type, expected []interface{} but got %T", actionItemSchema["anyOf"]) continue } @@ -170,7 +173,7 @@ func (p *Porter) injectMixinSchemas(ctx context.Context, manifestSchema jsonSche // Save the updated arrays into the json schema document mixinItemSchema["enum"] = mixinEnumSchema - return manifestSchema, err + return manifestSchema, span.Error(err) } func (p *Porter) GetReplacementSchema() (jsonSchema, error) { diff --git a/pkg/porter/stamp.go b/pkg/porter/stamp.go index 527a85d8b..015675b67 100644 --- a/pkg/porter/stamp.go +++ b/pkg/porter/stamp.go @@ -53,8 +53,11 @@ func (p *Porter) ensureLocalBundleIsUpToDate(ctx context.Context, opts bundleFil // IsBundleUpToDate checks the hash of the manifest against the hash in cnab/bundle.json. func (p *Porter) IsBundleUpToDate(ctx context.Context, opts bundleFileOptions) (bool, error) { + ctx, span := tracing.StartSpan(ctx) + defer span.EndSpan() + if opts.File == "" { - return false, errors.New("File is required") + return false, span.Error(errors.New("File is required")) } m, err := manifest.LoadManifestFrom(ctx, p.Config, opts.File) if err != nil { @@ -64,7 +67,7 @@ func (p *Porter) IsBundleUpToDate(ctx context.Context, opts bundleFileOptions) ( if exists, _ := p.FileSystem.Exists(opts.CNABFile); exists { bun, err := cnab.LoadBundle(p.Context, opts.CNABFile) if err != nil { - return false, fmt.Errorf("could not marshal data from %s: %w", opts.CNABFile, err) + return false, span.Error(fmt.Errorf("could not marshal data from %s: %w", opts.CNABFile, err)) } // Check whether invocation images exist in host registry. @@ -81,16 +84,14 @@ func (p *Porter) IsBundleUpToDate(ctx context.Context, opts bundleFileOptions) ( } if !isImageCached { - if p.Debug { - fmt.Fprintln(p.Err, fmt.Errorf("Invocation image %s doesn't exist in the local image cache, will need to build first", invocationImage.Image)) - } + span.Debugf("Invocation image %s doesn't exist in the local image cache, will need to build first", invocationImage.Image) return false, nil } } oldStamp, err := configadapter.LoadStamp(bun) if err != nil { - return false, fmt.Errorf("could not load stamp from %s: %w", opts.CNABFile, err) + return false, span.Error(fmt.Errorf("could not load stamp from %s: %w", opts.CNABFile, err)) } mixins, err := p.getUsedMixins(ctx, m) @@ -101,9 +102,7 @@ func (p *Porter) IsBundleUpToDate(ctx context.Context, opts bundleFileOptions) ( converter := configadapter.NewManifestConverter(p.Config, m, nil, mixins) newDigest, err := converter.DigestManifest() if err != nil { - if p.Debug { - fmt.Fprintln(p.Err, fmt.Errorf("could not determine if the bundle is up-to-date so will rebuild just in case: %w", err)) - } + span.Debugf("could not determine if the bundle is up-to-date so will rebuild just in case: %w", err) return false, nil } return oldStamp.ManifestDigest == newDigest, nil diff --git a/pkg/porter/storage.go b/pkg/porter/storage.go index b381665d5..839b33c73 100644 --- a/pkg/porter/storage.go +++ b/pkg/porter/storage.go @@ -10,6 +10,7 @@ import ( "get.porter.sh/porter/pkg" "get.porter.sh/porter/pkg/storage" + "get.porter.sh/porter/pkg/tracing" "github.com/hashicorp/go-multierror" ) @@ -40,13 +41,16 @@ func (p *Porter) MigrateStorage(ctx context.Context, opts MigrateStorageOptions) return p.Storage.Migrate(ctx, migrateOpts) } -func (p *Porter) FixPermissions() error { +func (p *Porter) FixPermissions(ctx context.Context) error { + ctx, span := tracing.StartSpan(ctx) + defer span.EndSpan() + home, err := p.GetHomeDir() if err != nil { return err } - fmt.Fprintf(p.Out, "Resetting file permissions in %s...\n", home) + span.Infof("Resetting file permissions in %s", home) // Fix as many files as we can, and then report any errors fixFile := func(path string, mode os.FileMode) error { @@ -55,22 +59,22 @@ func (p *Porter) FixPermissions() error { if os.IsNotExist(err) { return nil } else { - return fmt.Errorf("error checking file permissions for %s: %w", path, err) + return span.Error(fmt.Errorf("error checking file permissions for %s: %w", path, err)) } } if info.IsDir() { - return fmt.Errorf("fixFile was called on a directory %s", path) + return span.Error(fmt.Errorf("fixFile was called on a directory %s", path)) } if _, err = filepath.Rel(home, path); err != nil { - return fmt.Errorf("fixFile was called on a path, %s, that isn't in the PORTER_HOME directory %s", path, home) + return span.Error(fmt.Errorf("fixFile was called on a path, %s, that isn't in the PORTER_HOME directory %s", path, home)) } gotPerms := info.Mode().Perm() if mode != gotPerms|mode { if err := p.FileSystem.Chmod(path, mode); err != nil { - return fmt.Errorf("could not set permissions on file %s to %o: %w", path, mode, err) + return span.Error(fmt.Errorf("could not set permissions on file %s to %o: %w", path, mode, err)) } } return nil @@ -118,7 +122,7 @@ func (p *Porter) FixPermissions() error { } } - porterPath, _ := p.GetPorterPath() + porterPath, _ := p.GetPorterPath(ctx) binFiles := []string{porterPath} for _, file := range binFiles { if err := fixFile(file, pkg.FileModeExecutable); err != nil { @@ -133,5 +137,5 @@ func (p *Porter) FixPermissions() error { } } - return bigErr.ErrorOrNil() + return span.Error(bigErr.ErrorOrNil()) } diff --git a/pkg/porter/version.go b/pkg/porter/version.go index e8ced1c1f..3a9acbafa 100644 --- a/pkg/porter/version.go +++ b/pkg/porter/version.go @@ -12,6 +12,7 @@ import ( "get.porter.sh/porter/pkg/pkgmgmt" "get.porter.sh/porter/pkg/porter/version" "get.porter.sh/porter/pkg/printer" + "get.porter.sh/porter/pkg/tracing" ) type VersionOpts struct { @@ -73,13 +74,13 @@ func getSystemInfo() *SystemInfo { } func (p *Porter) PrintDebugInfo(ctx context.Context, opts VersionOpts, metadata pkgmgmt.Metadata) error { + log := tracing.LoggerFromContext(ctx) + opts.RawFormat = string(printer.FormatPlaintext) sysInfo := getSystemInfo() mixins, err := p.ListMixins(ctx) if err != nil { - if p.Debug { - fmt.Fprint(p.Err, err.Error()) - } + log.Debug(err.Error()) return nil } @@ -105,11 +106,11 @@ Mixins ` tmpl, err := template.New("systemDebugInfo").Parse(plaintextTmpl) if err != nil { - return fmt.Errorf("Failed to parse plaintext template: %w", err) + return log.Error(fmt.Errorf("Failed to parse plaintext template: %w", err)) } err = tmpl.Execute(p.Out, sysDebugInfo) - return err + return log.Error(err) default: - return fmt.Errorf("unsupported format: %s", opts.Format) + return log.Error(fmt.Errorf("unsupported format: %s", opts.Format)) } } diff --git a/tests/integration/pull_test.go b/tests/integration/pull_test.go index 3ecee57ff..2385de414 100644 --- a/tests/integration/pull_test.go +++ b/tests/integration/pull_test.go @@ -16,14 +16,14 @@ func TestPull_ContentDigestMissing(t *testing.T) { p := porter.NewTestPorter(t) defer p.Close() - p.SetupIntegrationTest() + ctx := p.SetupIntegrationTest() p.Debug = false opts := porter.BundlePullOptions{} opts.Reference = "getporterci/mysql:no-content-digest" require.NoError(t, opts.Validate()) - cachedBun, err := p.PullBundle(opts) + cachedBun, err := p.PullBundle(ctx, opts) require.Contains(t, err.Error(), "unable to verify that the pulled image getporterci/mysql-installer:no-content-digest is the invocation image referenced by the bundle because the bundle does not specify a content digest. This could allow for the invocation image to be replaced or tampered with") require.Equal(t, cache.CachedBundle{}, cachedBun) diff --git a/tests/integration/uninstall_test.go b/tests/integration/uninstall_test.go index 9b4aabb6f..047368843 100644 --- a/tests/integration/uninstall_test.go +++ b/tests/integration/uninstall_test.go @@ -1,5 +1,4 @@ //go:build integration -// +build integration package integration @@ -13,8 +12,6 @@ import ( ) func TestUninstall_DeleteInstallation(t *testing.T) { - t.Parallel() - test, err := tester.NewTest(t) defer test.Close() require.NoError(t, err, "test setup failed")