From 57b80e2d9cf46fc5bf4051a512145c1b4c728537 Mon Sep 17 00:00:00 2001 From: Kim Christensen <2461567+kichristensen@users.noreply.github.com> Date: Sat, 27 Apr 2024 19:46:06 +0200 Subject: [PATCH 1/3] Publish canary builds (#3084) * Publish canary builds Signed-off-by: Kim Christensen --------- Signed-off-by: Kim Christensen --- .github/workflows/porter-canary.yml | 19 ++--------------- .../workflows/porter-integration-release.yml | 21 +++++++++++++++++++ 2 files changed, 23 insertions(+), 17 deletions(-) diff --git a/.github/workflows/porter-canary.yml b/.github/workflows/porter-canary.yml index eda08cb28..2b67a6185 100644 --- a/.github/workflows/porter-canary.yml +++ b/.github/workflows/porter-canary.yml @@ -1,29 +1,14 @@ name: porter/porter-canary on: - workflow_dispatch: - inputs: - shouldPublish: - description: Should Publish - default: true - type: boolean - required: false - skipTests: - description: Skip Tests - default: false - type: boolean - required: false push: branches: - main - release/* - pull_request: - branches: - - split-builds jobs: build_pipelinesrelease_template: name: build_pipelinesrelease_template uses: ./.github/workflows/build_pipelinesrelease_template.yml with: registry: ghcr.io/getporter - shouldPublish: "${{inputs.shouldPublish}}" - skipTests: "${{inputs.skipTests}}" + shouldPublish: true + skipTests: false diff --git a/.github/workflows/porter-integration-release.yml b/.github/workflows/porter-integration-release.yml index 19b59b1ad..95634fc29 100644 --- a/.github/workflows/porter-integration-release.yml +++ b/.github/workflows/porter-integration-release.yml @@ -12,107 +12,128 @@ env: jobs: archive_integration_test: + name: Archive Integration Test uses: getporter/porter/.github/workflows/integ-reuseable-workflow.yml@main with: test_name: archive_test registry: ${{inputs.registry}} build_integration_test: + name: Build Integration Test uses: getporter/porter/.github/workflows/integ-reuseable-workflow.yml@main with: test_name: build_test registry: ${{inputs.registry}} cli_integration_test: + name: CLI Integration Test uses: getporter/porter/.github/workflows/integ-reuseable-workflow.yml@main with: test_name: cli_test registry: ${{inputs.registry}} connection_nix_integration_test: + name: Connection Integration Test uses: getporter/porter/.github/workflows/integ-reuseable-workflow.yml@main with: test_name: connection_nix_test registry: ${{inputs.registry}} copy_integration_test: + name: Copy Integration Test uses: getporter/porter/.github/workflows/integ-reuseable-workflow.yml@main with: test_name: copy_test registry: ${{inputs.registry}} dependenciesv1_integration_test: + name: Dependencies V1 Integration Test uses: getporter/porter/.github/workflows/integ-reuseable-workflow.yml@main with: test_name: dependenciesv1_test registry: ${{inputs.registry}} dependenciesv2_integration_test: + name: Dependencies V2 Integration Test uses: getporter/porter/.github/workflows/integ-reuseable-workflow.yml@main with: test_name: dependenciesv2_test registry: ${{inputs.registry}} driver_integration_test: + name: Driver Integration Test uses: getporter/porter/.github/workflows/integ-reuseable-workflow.yml@main with: test_name: driver_test registry: ${{inputs.registry}} install_integration_test: + name: Install Integration Test uses: getporter/porter/.github/workflows/integ-reuseable-workflow.yml@main with: test_name: install_test registry: ${{inputs.registry}} invoke_integration_test: + name: Invoke Integration Test uses: getporter/porter/.github/workflows/integ-reuseable-workflow.yml@main with: test_name: invoke_test registry: ${{inputs.registry}} lint_integration_test: + name: Lint Integration Test uses: getporter/porter/.github/workflows/integ-reuseable-workflow.yml@main with: test_name: lint_test registry: ${{inputs.registry}} migration_integration_test: + name: Migration Integration Test uses: getporter/porter/.github/workflows/integ-reuseable-workflow.yml@main with: test_name: migration_test registry: ${{inputs.registry}} outputs_integration_test: + name: Outputs Integration Test uses: getporter/porter/.github/workflows/integ-reuseable-workflow.yml@main with: test_name: outputs_test registry: ${{inputs.registry}} publish_integration_test: + name: Publish Integration Test uses: getporter/porter/.github/workflows/integ-reuseable-workflow.yml@main with: test_name: publish_test registry: ${{inputs.registry}} pull_integration_test: + name: Pull Integration Test uses: getporter/porter/.github/workflows/integ-reuseable-workflow.yml@main with: test_name: pull_test registry: ${{inputs.registry}} registry_integration_test: + name: Registry Integration Test uses: getporter/porter/.github/workflows/integ-reuseable-workflow.yml@main with: test_name: registry_integration_test registry: ${{inputs.registry}} schema_integration_test: + name: Schema Integration Test uses: getporter/porter/.github/workflows/integ-reuseable-workflow.yml@main with: test_name: schema_test registry: ${{inputs.registry}} sensitive_data_integration_test: + name: Sensitive data Integration Test uses: getporter/porter/.github/workflows/integ-reuseable-workflow.yml@main with: test_name: sensitive_data_test registry: ${{inputs.registry}} suppress_output_integration_test: + name: Suppress output Integration Test uses: getporter/porter/.github/workflows/integ-reuseable-workflow.yml@main with: test_name: suppress_output_test registry: ${{inputs.registry}} telemetry_test: + name: Telemetry Integration Test uses: getporter/porter/.github/workflows/integ-reuseable-workflow.yml@main with: test_name: telemetry_test registry: ${{inputs.registry}} # Reusable workflows only supports 20 jobs uninstall_test_integ: + name: Uninstall Integration Test runs-on: ubuntu-latest steps: - name: checkout From 9f85030c8f25be3cbf8f5faf2535978de2d747b2 Mon Sep 17 00:00:00 2001 From: Kim Christensen <2461567+kichristensen@users.noreply.github.com> Date: Mon, 29 Apr 2024 17:51:37 +0200 Subject: [PATCH 2/3] Perform full checkout of repository during release (#3088) Full checkout of repository during release Otherwise `git describe --tags` won't work correctly, because of the missing history, causing the canary build to not be published. Signed-off-by: Kim Christensen --- .github/workflows/build_pipelinesrelease_template.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/build_pipelinesrelease_template.yml b/.github/workflows/build_pipelinesrelease_template.yml index f443fbd0b..5130de532 100644 --- a/.github/workflows/build_pipelinesrelease_template.yml +++ b/.github/workflows/build_pipelinesrelease_template.yml @@ -167,6 +167,8 @@ jobs: steps: - name: checkout uses: actions/checkout@v4.1.0 + with: + fetch-depth: 0 - uses: actions/setup-go@v4 with: go-version: "${{ inputs.GOVERSION }}" From 7422f37eaa1a7d958f46acf675ea0b301b6d3471 Mon Sep 17 00:00:00 2001 From: Kim Christensen <2461567+kichristensen@users.noreply.github.com> Date: Mon, 29 Apr 2024 20:36:45 +0200 Subject: [PATCH 3/3] Execute bundle from a pre-computed Run (#3051) "Merge" the changes from Carolyns PR #2722 Signed-off-by: Kim Christensen --- pkg/cnab/provider/action.go | 80 +++++---------- pkg/cnab/provider/credentials.go | 56 ++++------- pkg/cnab/provider/credentials_test.go | 104 +++++++++----------- pkg/cnab/provider/docker_linux_test.go | 2 +- pkg/cnab/provider/helpers.go | 2 +- pkg/cnab/provider/parameters.go | 38 +++++++ pkg/cnab/provider/runtime.go | 3 +- pkg/porter/dependencies.go | 9 +- pkg/porter/lifecycle.go | 85 +++++++++++++++- pkg/porter/lifecycle_test.go | 4 + pkg/porter/parameters.go | 12 ++- pkg/porter/porter.go | 2 +- pkg/porter/reconcile_test.go | 21 ++-- pkg/porter/show_test.go | 13 ++- pkg/porter/testdata/test-creds/mycreds.yaml | 8 ++ pkg/secrets/strategy.go | 17 ---- pkg/storage/credentialset.go | 54 ++++++++-- pkg/storage/credentialset_test.go | 22 +++-- pkg/storage/migrations/migration.go | 28 +++--- pkg/storage/parameterset.go | 34 +++++++ pkg/storage/parameterset_test.go | 44 +++++++++ pkg/storage/run.go | 72 +++++++++++++- pkg/storage/run_test.go | 9 +- pkg/storage/testdata/marshaled_run.json | 2 +- 24 files changed, 485 insertions(+), 236 deletions(-) create mode 100644 pkg/cnab/provider/parameters.go create mode 100644 pkg/porter/testdata/test-creds/mycreds.yaml diff --git a/pkg/cnab/provider/action.go b/pkg/cnab/provider/action.go index 75940d366..ccbc43ae6 100644 --- a/pkg/cnab/provider/action.go +++ b/pkg/cnab/provider/action.go @@ -5,7 +5,6 @@ import ( "encoding/json" "errors" "fmt" - "sort" "get.porter.sh/porter/pkg/cnab" "get.porter.sh/porter/pkg/config" @@ -24,14 +23,14 @@ type HostVolumeMountSpec struct { ReadOnly bool } -// Shared arguments for all CNAB actions +// ActionArguments are the shared arguments for all bundle runs. type ActionArguments struct { - // Action to execute, e.g. install, upgrade. - Action string - // Name of the installation. Installation storage.Installation + // Run defines how to execute the bundle. + Run storage.Run + // BundleReference is the set of information necessary to execute a bundle. BundleReference cnab.BundleReference @@ -40,6 +39,7 @@ type ActionArguments struct { Files map[string]string // Params is the fully resolved set of parameters. + // TODO(PEP003): This should be removed in https://github.com/getporter/porter/issues/2699 Params map[string]interface{} // Driver is the CNAB-compliant driver used to run bundle actions. @@ -145,56 +145,57 @@ func (r *Runtime) Execute(ctx context.Context, args ActionArguments) error { case <-ctx.Done(): return ctx.Err() default: + currentRun := args.Run ctx, log := tracing.StartSpan(ctx, - attribute.String("action", args.Action), + attribute.String("action", currentRun.Action), attribute.Bool("allowDockerHostAccess", args.AllowDockerHostAccess), attribute.String("driver", args.Driver)) defer log.EndSpan() args.BundleReference.AddToTrace(ctx) args.Installation.AddToTrace(ctx) - if args.Action == "" { + if currentRun.Action == "" { return log.Error(errors.New("action is required")) } b, err := r.ProcessBundle(ctx, args.BundleReference.Definition) if err != nil { - return log.Error(err) - } - - currentRun, err := r.CreateRun(ctx, args, b) - if err != nil { - return log.Error(err) + return err } // Validate the action if _, err := b.GetAction(currentRun.Action); err != nil { - return log.Error(fmt.Errorf("invalid action '%s' specified for bundle %s: %w", currentRun.Action, b.Name, err)) - } - - creds, err := r.loadCredentials(ctx, b, args) - if err != nil { - return log.Error(fmt.Errorf("not load credentials: %w", err)) + return log.Errorf("invalid action '%s' specified for bundle %s: %w", currentRun.Action, b.Name, err) } log.Debugf("Using runtime driver %s\n", args.Driver) driver, err := r.newDriver(args.Driver, args) if err != nil { - return log.Error(fmt.Errorf("unable to instantiate driver: %w", err)) + return log.Errorf("unable to instantiate driver: %w", err) } a := cnabaction.New(driver) a.SaveLogs = args.PersistLogs + // Resolve parameters and credentials just-in-time (JIT) before running the bundle, do this at the *LAST* possible moment + log.Info("Just-in-time resolving credentials...") + if err = r.loadCredentials(ctx, b, ¤tRun); err != nil { + return log.Errorf("could not resolve credentials before running the bundle: %w", err) + } + log.Info("Just-in-time resolving parameters...") + if err = r.loadParameters(ctx, b, ¤tRun); err != nil { + return log.Errorf("could not resolve parameters before running the bundle: %w", err) + } + if currentRun.ShouldRecord() { err = r.SaveRun(ctx, args.Installation, currentRun, cnab.StatusRunning) if err != nil { - return log.Error(fmt.Errorf("could not save the pending action's status, the bundle was not executed: %w", err)) + return log.Errorf("could not save the pending action's status, the bundle was not executed: %w", err) } } cnabClaim := currentRun.ToCNAB() - cnabCreds := creds.ToCNAB() + cnabCreds := currentRun.Credentials.ToCNAB() // The claim and credentials contain sensitive values. Only trace it in special dev builds (nothing is traced for release builds) log.SetSensitiveAttributes( tracing.ObjectAttribute("cnab-claim", cnabClaim), @@ -204,46 +205,19 @@ func (r *Runtime) Execute(ctx context.Context, args ActionArguments) error { if currentRun.ShouldRecord() { if err != nil { err = r.appendFailedResult(ctx, err, currentRun) - return log.Error(fmt.Errorf("failed to record that %s for installation %s failed: %w", args.Action, args.Installation.Name, err)) + return log.Errorf("failed to record that %s for installation %s failed: %w", currentRun.Action, args.Installation.Name, err) } return r.SaveOperationResult(ctx, opResult, args.Installation, currentRun, currentRun.NewResultFrom(result)) } if err != nil { - return log.Error(fmt.Errorf("execution of %s for installation %s failed: %w", args.Action, args.Installation.Name, err)) + return log.Errorf("execution of %s for installation %s failed: %w", currentRun.Action, args.Installation.Name, err) } return nil } } -func (r *Runtime) CreateRun(ctx context.Context, args ActionArguments, b cnab.ExtendedBundle) (storage.Run, error) { - ctx, span := tracing.StartSpan(ctx) - defer span.EndSpan() - - // Create a record for the run we are about to execute - var currentRun = args.Installation.NewRun(args.Action, b) - currentRun.Bundle = b.Bundle - currentRun.BundleReference = args.BundleReference.Reference.String() - currentRun.BundleDigest = args.BundleReference.Digest.String() - - var err error - extb := cnab.NewBundle(b.Bundle) - currentRun.Parameters.Parameters, err = r.sanitizer.CleanRawParameters(ctx, args.Params, extb, currentRun.ID) - if err != nil { - return storage.Run{}, span.Error(err) - } - - // TODO: Do not save secrets when the run isn't recorded - currentRun.ParameterOverrides = storage.LinkSensitiveParametersToSecrets(currentRun.ParameterOverrides, extb, currentRun.ID) - currentRun.CredentialSets = args.Installation.CredentialSets - sort.Strings(currentRun.CredentialSets) - - currentRun.ParameterSets = args.Installation.ParameterSets - sort.Strings(currentRun.ParameterSets) - return currentRun, nil -} - // SaveRun with the specified status. func (r *Runtime) SaveRun(ctx context.Context, installation storage.Installation, run storage.Run, status string) error { ctx, span := tracing.StartSpan(ctx) @@ -259,12 +233,12 @@ func (r *Runtime) SaveRun(ctx context.Context, installation storage.Installation return span.Error(fmt.Errorf("error saving the installation record before executing the bundle: %w", err)) } - result := run.NewResult(status) - err = r.installations.InsertRun(ctx, run) + err = r.installations.UpsertRun(ctx, run) if err != nil { return span.Error(fmt.Errorf("error saving the installation run record before executing the bundle: %w", err)) } + result := run.NewResult(status) err = r.installations.InsertResult(ctx, result) if err != nil { return span.Error(fmt.Errorf("error saving the installation status record before executing the bundle: %w", err)) diff --git a/pkg/cnab/provider/credentials.go b/pkg/cnab/provider/credentials.go index 94faab660..44cec328f 100644 --- a/pkg/cnab/provider/credentials.go +++ b/pkg/cnab/provider/credentials.go @@ -4,52 +4,34 @@ import ( "context" "get.porter.sh/porter/pkg/cnab" - "get.porter.sh/porter/pkg/secrets" "get.porter.sh/porter/pkg/storage" "get.porter.sh/porter/pkg/tracing" - "go.mongodb.org/mongo-driver/bson" ) -func (r *Runtime) loadCredentials(ctx context.Context, b cnab.ExtendedBundle, args ActionArguments) (secrets.Set, error) { +func (r *Runtime) loadCredentials(ctx context.Context, b cnab.ExtendedBundle, run *storage.Run) error { ctx, span := tracing.StartSpan(ctx) defer span.EndSpan() - if len(args.Installation.CredentialSets) == 0 { - return nil, storage.Validate(nil, b.Credentials, args.Action) + resolvedCredentials, err := r.credentials.ResolveAll(ctx, run.Credentials) + if err != nil { + return span.Error(err) } - // The strategy here is "last one wins". We loop through each credential file and - // calculate its credentials. Then we insert them into the creds map in the order - // in which they were supplied on the CLI. - resolvedCredentials := secrets.Set{} - for _, name := range args.Installation.CredentialSets { - var cset storage.CredentialSet - // Try to get the creds in the local namespace first, fallback to the global creds - query := storage.FindOptions{ - Sort: []string{"-namespace"}, - Filter: bson.M{ - "name": name, - "$or": []bson.M{ - {"namespace": ""}, - {"namespace": args.Installation.Namespace}, - }, - }, - } - store := r.credentials.GetDataStore() - err := store.FindOne(ctx, storage.CollectionCredentials, query, &cset) - if err != nil { - return nil, err - } - - rc, err := r.credentials.ResolveAll(ctx, cset) - if err != nil { - return nil, err - } - - for k, v := range rc { - resolvedCredentials[k] = v - } + for i, cred := range run.Credentials.Credentials { + run.Credentials.Credentials[i].ResolvedValue = resolvedCredentials[cred.Name] } - return resolvedCredentials, storage.Validate(resolvedCredentials, b.Credentials, args.Action) + err = run.Credentials.ValidateBundle(b.Credentials, run.Action) + if err != nil { + return span.Error(err) + } + + err = run.SetCredentialsDigest() + if err != nil { + // Just warn since the digest isn't critical for running the bundle + // If it's not set properly, we will recalculate as needed + span.Warnf("WARNING: unable to set the run's credentials digest: %w", err) + } + + return nil } diff --git a/pkg/cnab/provider/credentials_test.go b/pkg/cnab/provider/credentials_test.go index a4f90e2e3..d4aac3d86 100644 --- a/pkg/cnab/provider/credentials_test.go +++ b/pkg/cnab/provider/credentials_test.go @@ -8,6 +8,7 @@ import ( "get.porter.sh/porter/pkg/secrets" "get.porter.sh/porter/pkg/storage" "github.com/cnabio/cnab-go/bundle" + "github.com/cnabio/cnab-go/valuesource" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -21,19 +22,6 @@ func TestRuntime_loadCredentials(t *testing.T) { r.TestCredentials.AddSecret("password", "mypassword") r.TestCredentials.AddSecret("db-password", "topsecret") - r.TestConfig.TestContext.AddTestFile("testdata/db-creds.json", "/db-creds.json") - - cs1 := storage.NewCredentialSet("", "mycreds", secrets.SourceMap{ - Name: "password", - Source: secrets.Source{ - Strategy: secrets.SourceSecret, - Hint: "password", - }, - }) - - err := r.credentials.InsertCredentialSet(context.Background(), cs1) - require.NoError(t, err, "Save credential set failed") - b := cnab.NewBundle(bundle.Bundle{ Credentials: map[string]bundle.Credential{ "password": { @@ -49,28 +37,27 @@ func TestRuntime_loadCredentials(t *testing.T) { }, }) - args := ActionArguments{ - Installation: storage.Installation{ - InstallationSpec: storage.InstallationSpec{ - CredentialSets: []string{"mycreds"}}, - }, - Action: "install"} - gotValues, err := r.loadCredentials(context.Background(), b, args) + run := storage.Run{ + Action: cnab.ActionInstall, + Credentials: storage.NewInternalCredentialSet(secrets.SourceMap{ + Name: "password", + Source: secrets.Source{ + Strategy: secrets.SourceSecret, + Hint: "password", + }, + }), + } + err := r.loadCredentials(context.Background(), b, &run) require.NoError(t, err, "loadCredentials failed") + require.Equal(t, "sha256:2d6d3c91ef272afeef2bb29b2fb4b1670c756c623195e71916b8ee138fba60cb", + run.CredentialsDigest, "expected loadCredentials to set the digest of resolved credentials") + require.NotEmpty(t, run.Credentials.Credentials[0].ResolvedValue, "expected loadCredentials to set the resolved value of the credentials on the Run") - wantValues := secrets.Set{ + gotValues := run.Credentials.ToCNAB() + wantValues := valuesource.Set{ "password": "mypassword", } assert.Equal(t, wantValues, gotValues, "resolved unexpected credential values") - - args = ActionArguments{ - Installation: storage.Installation{ - InstallationSpec: storage.InstallationSpec{ - CredentialSets: []string{"/db-creds.json"}}, - }, - Action: "install"} - _, err = r.loadCredentials(context.Background(), b, args) - require.Error(t, err, "loadCredentials should not load from a file") } func TestRuntime_loadCredentials_WithApplyTo(t *testing.T) { @@ -94,13 +81,13 @@ func TestRuntime_loadCredentials_WithApplyTo(t *testing.T) { r := NewTestRuntime(t) defer r.Close() - args := ActionArguments{Action: "status"} + run := &storage.Run{Action: "status"} b := getBundle(true) - gotValues, err := r.loadCredentials(context.Background(), b, args) + err := r.loadCredentials(context.Background(), b, run) require.NoError(t, err, "loadCredentials failed") - var wantValues secrets.Set - assert.Equal(t, wantValues, gotValues) + gotValues := run.Credentials.ToCNAB() + assert.Empty(t, gotValues) }) t.Run("optional credential missing", func(t *testing.T) { @@ -108,13 +95,13 @@ func TestRuntime_loadCredentials_WithApplyTo(t *testing.T) { r := NewTestRuntime(t) defer r.Close() - args := ActionArguments{Action: "install"} + run := &storage.Run{Action: "install"} b := getBundle(false) - gotValues, err := r.loadCredentials(context.Background(), b, args) + err := r.loadCredentials(context.Background(), b, run) require.NoError(t, err, "loadCredentials failed") - var wantValues secrets.Set - assert.Equal(t, wantValues, gotValues) + gotValues := run.Credentials.ToCNAB() + assert.Empty(t, gotValues) }) t.Run("required credential missing", func(t *testing.T) { @@ -122,9 +109,9 @@ func TestRuntime_loadCredentials_WithApplyTo(t *testing.T) { r := NewTestRuntime(t) defer r.Close() - args := ActionArguments{Action: "install"} + run := &storage.Run{Action: "install"} b := getBundle(true) - _, err := r.loadCredentials(context.Background(), b, args) + err := r.loadCredentials(context.Background(), b, run) require.Error(t, err, "expected the credential to be required") }) @@ -135,27 +122,26 @@ func TestRuntime_loadCredentials_WithApplyTo(t *testing.T) { r.TestCredentials.AddSecret("password", "mypassword") - cs1 := storage.NewCredentialSet("", "mycreds", secrets.SourceMap{ - Name: "password", - Source: secrets.Source{ - Strategy: secrets.SourceSecret, - Hint: "password", - }, - }) - - err := r.credentials.InsertCredentialSet(context.Background(), cs1) - require.NoError(t, err, "Save credential set failed") - b := getBundle(true) - args := ActionArguments{ - Installation: storage.Installation{ - InstallationSpec: storage.InstallationSpec{ - CredentialSets: []string{"mycreds"}}, - }, - Action: "install"} - gotValues, err := r.loadCredentials(context.Background(), b, args) + run := &storage.Run{ + Action: cnab.ActionInstall, + CredentialSets: []string{"mycreds"}, + Credentials: storage.NewInternalCredentialSet(secrets.SourceMap{ + Name: "password", + Source: secrets.Source{ + Strategy: secrets.SourceSecret, + Hint: "password", + }, + }), + } + err := r.loadCredentials(context.Background(), b, run) require.NoError(t, err, "loadCredentials failed") - assert.Equal(t, secrets.Set{"password": "mypassword"}, gotValues) + require.Equal(t, "sha256:2d6d3c91ef272afeef2bb29b2fb4b1670c756c623195e71916b8ee138fba60cb", + run.CredentialsDigest, "expected loadCredentials to set the digest of resolved credentials") + require.NotEmpty(t, run.Credentials.Credentials[0].ResolvedValue, "expected loadCredentials to set the resolved value of the credentials on the Run") + + gotValues := run.Credentials.ToCNAB() + assert.Equal(t, valuesource.Set{"password": "mypassword"}, gotValues, "incorrect resolved credentials") }) } diff --git a/pkg/cnab/provider/docker_linux_test.go b/pkg/cnab/provider/docker_linux_test.go index 2305c3e7a..fbec44af3 100644 --- a/pkg/cnab/provider/docker_linux_test.go +++ b/pkg/cnab/provider/docker_linux_test.go @@ -14,7 +14,7 @@ func TestRuntime_getDockerGroupID(t *testing.T) { cfg.Setenv(test.ExpectedCommandEnv, "getent group docker") cfg.Setenv(test.ExpectedCommandOutputEnv, "docker:x:103") - r := NewRuntime(cfg.Config, nil, nil, nil, nil) + r := NewRuntime(cfg.Config, nil, nil, nil, nil, nil) gid, err := r.getDockerGroupId() require.NoError(t, err) assert.Equal(t, "103", gid) diff --git a/pkg/cnab/provider/helpers.go b/pkg/cnab/provider/helpers.go index 84ea423d7..b8baf6460 100644 --- a/pkg/cnab/provider/helpers.go +++ b/pkg/cnab/provider/helpers.go @@ -40,7 +40,7 @@ func NewTestRuntime(t *testing.T) *TestRuntime { func NewTestRuntimeFor(tc *config.TestConfig, testInstallations *storage.TestInstallationProvider, testCredentials *storage.TestCredentialSetProvider, testParameters *storage.TestParameterSetProvider, testSecrets secrets.Store) *TestRuntime { return &TestRuntime{ - Runtime: NewRuntime(tc.Config, testInstallations, testCredentials, testSecrets, storage.NewSanitizer(testParameters, testSecrets)), + Runtime: NewRuntime(tc.Config, testInstallations, testCredentials, testParameters, testSecrets, storage.NewSanitizer(testParameters, testSecrets)), TestStorage: storage.TestStore{}, TestInstallations: testInstallations, TestCredentials: testCredentials, diff --git a/pkg/cnab/provider/parameters.go b/pkg/cnab/provider/parameters.go new file mode 100644 index 000000000..377bd84c5 --- /dev/null +++ b/pkg/cnab/provider/parameters.go @@ -0,0 +1,38 @@ +package cnabprovider + +import ( + "context" + + "get.porter.sh/porter/pkg/cnab" + "get.porter.sh/porter/pkg/storage" + "get.porter.sh/porter/pkg/tracing" +) + +// loadParameters resolves the prepared parameter set associated with the Run, and +// updates Run.Parameters with the resolved values. +func (r *Runtime) loadParameters(ctx context.Context, b cnab.ExtendedBundle, run *storage.Run) error { + ctx, span := tracing.StartSpan(ctx) + defer span.EndSpan() + + resolvedParameters, err := r.parameters.ResolveAll(ctx, run.Parameters) + if err != nil { + return err + } + + // Apply the resolved values back onto the run, these won't be persisted but are used in-memory + for i, param := range run.Parameters.Parameters { + run.Parameters.Parameters[i].ResolvedValue = resolvedParameters[param.Name] + } + + if err = run.Parameters.ValidateBundle(b.Parameters, run.Action); err != nil { + return span.Error(err) + } + + if err = run.SetParametersDigest(); err != nil { + // Just warn since the digest isn't critical for running the bundle + // If it's not set properly, we will recalculate as needed + span.Warnf("WARNING: unable to set the run's parameters digest: %w", err) + } + + return nil +} diff --git a/pkg/cnab/provider/runtime.go b/pkg/cnab/provider/runtime.go index 5e8557c71..805a24af3 100644 --- a/pkg/cnab/provider/runtime.go +++ b/pkg/cnab/provider/runtime.go @@ -22,11 +22,12 @@ type Runtime struct { Extensions cnab.ProcessedExtensions } -func NewRuntime(c *config.Config, installations storage.InstallationProvider, credentials storage.CredentialSetProvider, secrets secrets.Store, sanitizer *storage.Sanitizer) *Runtime { +func NewRuntime(c *config.Config, installations storage.InstallationProvider, credentials storage.CredentialSetProvider, parameters storage.ParameterSetProvider, secrets secrets.Store, sanitizer *storage.Sanitizer) *Runtime { return &Runtime{ Config: c, installations: installations, credentials: credentials, + parameters: parameters, secrets: secrets, sanitizer: sanitizer, Extensions: cnab.ProcessedExtensions{}, diff --git a/pkg/porter/dependencies.go b/pkg/porter/dependencies.go index 2c09a2251..fcb8c0769 100644 --- a/pkg/porter/dependencies.go +++ b/pkg/porter/dependencies.go @@ -407,14 +407,19 @@ func (e *dependencyExecutioner) runDependencyv2(ctx context.Context, dep *queued func (e *dependencyExecutioner) getActionArgs(ctx context.Context, dep *queuedDependency) error { - finalParams, err := e.porter.finalizeParameters(ctx, e.depArgs.Installation, dep.BundleReference.Definition, e.parentArgs.Action, dep.Parameters) + actionName := e.parentArgs.Run.Action + finalParams, err := e.porter.finalizeParameters(ctx, e.depArgs.Installation, dep.BundleReference.Definition, actionName, dep.Parameters) if err != nil { return fmt.Errorf("error resolving parameters for dependency %s: %w", dep.Alias, err) } + depRun, err := e.porter.createRun(ctx, dep.BundleReference, e.depArgs.Installation, actionName, finalParams) + if err != nil { + return fmt.Errorf("error creating run for dependency %s: %w", dep.Alias, err) + } e.depArgs = cnabprovider.ActionArguments{ BundleReference: dep.BundleReference, - Action: e.parentArgs.Action, Installation: e.depArgs.Installation, + Run: depRun, Driver: e.parentArgs.Driver, AllowDockerHostAccess: e.parentOpts.AllowDockerHostAccess, Params: finalParams, diff --git a/pkg/porter/lifecycle.go b/pkg/porter/lifecycle.go index 6d3e46350..ed022f90e 100644 --- a/pkg/porter/lifecycle.go +++ b/pkg/porter/lifecycle.go @@ -15,9 +15,11 @@ import ( cnabprovider "get.porter.sh/porter/pkg/cnab/provider" "get.porter.sh/porter/pkg/encoding" "get.porter.sh/porter/pkg/portercontext" + "get.porter.sh/porter/pkg/secrets" "get.porter.sh/porter/pkg/storage" "get.porter.sh/porter/pkg/tracing" "github.com/opencontainers/go-digest" + "go.mongodb.org/mongo-driver/bson" ) // BundleAction is an interface that defines a method for supplying @@ -341,8 +343,13 @@ func (p *Porter) BuildActionArgs(ctx context.Context, installation storage.Insta } } + run, err := p.createRun(ctx, bundleRef, installation, action.GetAction(), opts.GetParameters()) + if err != nil { + return cnabprovider.ActionArguments{}, err + } + args := cnabprovider.ActionArguments{ - Action: action.GetAction(), + Run: run, Installation: installation, BundleReference: bundleRef, Params: opts.GetParameters(), @@ -410,3 +417,79 @@ func (p *Porter) prepullBundleByReference(ctx context.Context, opts *BundleRefer return cachedBundle, nil } + +// createRun generates a Run record instructing porter exactly how to run the bundle +// and includes audit/status fields as well. +func (p *Porter) createRun(ctx context.Context, bundleRef cnab.BundleReference, inst storage.Installation, action string, params map[string]interface{}) (storage.Run, error) { + ctx, span := tracing.StartSpan(ctx) + defer span.EndSpan() + + // Create a record for the run we are about to execute + var currentRun = inst.NewRun(action, bundleRef.Definition) + currentRun.Bundle = bundleRef.Definition.Bundle + currentRun.BundleReference = bundleRef.Reference.String() + currentRun.BundleDigest = bundleRef.Digest.String() + + var err error + cleanParams, err := p.Sanitizer.CleanRawParameters(ctx, params, bundleRef.Definition, currentRun.ID) + if err != nil { + return storage.Run{}, span.Error(err) + } + currentRun.Parameters.Parameters = cleanParams + + // TODO: Do not save secrets when the run isn't recorded + currentRun.ParameterOverrides = storage.LinkSensitiveParametersToSecrets(currentRun.ParameterOverrides, bundleRef.Definition, currentRun.ID) + currentRun.ParameterSets = inst.ParameterSets + + // Persist an audit record of the credential sets used to determine the final + // credentials injected into the bundle. + // + // These should remain in the order specified on the installation, and not + // sorted, so that the last specified set overrides the one before it when a + // value is specified in more than one set. + currentRun.CredentialSets = inst.CredentialSets + + // Combine the credential sets above into a single credential set we can resolve just-in-time (JIT) before running the bundle. + finalCreds := make(map[string]secrets.SourceMap, len(currentRun.Bundle.Credentials)) + for _, csName := range currentRun.CredentialSets { + var cs storage.CredentialSet + // Try to get the creds in the local namespace first, fallback to the global creds + query := storage.FindOptions{ + Sort: []string{"-namespace"}, + Filter: bson.M{ + "name": csName, + "$or": []bson.M{ + {"namespace": ""}, + {"namespace": currentRun.Namespace}, + }, + }, + } + store := p.Credentials.GetDataStore() + err := store.FindOne(ctx, storage.CollectionCredentials, query, &cs) + if err != nil { + return storage.Run{}, span.Errorf("could not find credential set named %s in the %s namespace or global namespace: %w", csName, inst.Namespace, err) + } + + for _, cred := range cs.Credentials { + credDef, ok := currentRun.Bundle.Credentials[cred.Name] + if !ok || !credDef.AppliesTo(currentRun.Action) { + // ignore extra credential mappings in the set that are not defined by the bundle or used by the current action + // it's okay to over specify so that people can reuse sets better + continue + } + + // If a credential is mapped in multiple credential sets, the strategy associated with the last set specified "wins" + finalCreds[cred.Name] = cred + } + } + + if len(finalCreds) > 0 { + // Store the composite credential set on the run, so that the runtime can later resolve them in a single step + currentRun.Credentials = storage.NewInternalCredentialSet() + for _, cred := range finalCreds { + currentRun.Credentials.Credentials = append(currentRun.Credentials.Credentials, cred) + } + } + + return currentRun, nil +} diff --git a/pkg/porter/lifecycle_test.go b/pkg/porter/lifecycle_test.go index 031e62315..5186b2e6b 100644 --- a/pkg/porter/lifecycle_test.go +++ b/pkg/porter/lifecycle_test.go @@ -5,6 +5,7 @@ import ( "fmt" "io" "runtime" + "sort" "strings" "testing" @@ -99,6 +100,8 @@ func TestPorter_BuildActionArgs(t *testing.T) { p := NewTestPorter(t) p.TestConfig.TestContext.AddTestFile("testdata/porter.yaml", "porter.yaml") p.TestConfig.TestContext.AddTestFileFromRoot("pkg/runtime/testdata/relocation-mapping.json", "relocation-mapping.json") + p.TestCredentials.AddTestCredentials("testdata/test-creds/mycreds.yaml") + opts := InstallOptions{ BundleExecutionOptions: &BundleExecutionOptions{ AllowDockerHostAccess: true, @@ -475,6 +478,7 @@ func TestPorter_applyActionOptionsToInstallation_sanitizesParameters(t *testing. require.NoError(t, err) // Check that when no parameter overrides are specified, we use the originally specified parameters from the previous run + sort.Sort(i.Parameters.Parameters) require.Len(t, i.Parameters.Parameters, 2) require.Equal(t, "my-first-param", i.Parameters.Parameters[0].Name) require.Equal(t, "1", i.Parameters.Parameters[0].Source.Hint) diff --git a/pkg/porter/parameters.go b/pkg/porter/parameters.go index 897db5c02..c2f27445f 100644 --- a/pkg/porter/parameters.go +++ b/pkg/porter/parameters.go @@ -776,9 +776,15 @@ func (p *Porter) CreateParameter(opts ParameterCreateOptions) error { } } -// applyActionOptionsToInstallation applies the specified action (e.g. install/upgrade) to an installation record. -// This resolves the parameters to their final form to be passed to the CNAB runtime, and modifies the specified installation record. -// You must sanitize the parameters before saving the installation so that sensitive values are not saved to the database. +// applyActionOptionsToInstallation applies the specified action (e.g. +// install/upgrade) to an installation record. This consolidates parameters and +// credentials into a single parameter set or credential set, ready to be resolved +// immediately before the bundle is run, and modifies the specified installation +// record. +// +// This does not resolve the parameters, that only occurs before the bundle is run. +// You must sanitize the parameters before saving the installation so +// that sensitive values are not saved to the database. func (p *Porter) applyActionOptionsToInstallation(ctx context.Context, ba BundleAction, inst *storage.Installation) error { ctx, span := tracing.StartSpan(ctx) defer span.EndSpan() diff --git a/pkg/porter/porter.go b/pkg/porter/porter.go index 20f007ad6..874752dab 100644 --- a/pkg/porter/porter.go +++ b/pkg/porter/porter.go @@ -78,7 +78,7 @@ func NewFor(c *config.Config, store storage.Store, secretStorage secrets.Store) Templates: templates.NewTemplates(c), Mixins: mixin.NewPackageManager(c), Plugins: plugins.NewPackageManager(c), - CNAB: cnabprovider.NewRuntime(c, installationStorage, credStorage, secretStorage, sanitizerService), + CNAB: cnabprovider.NewRuntime(c, installationStorage, credStorage, paramStorage, secretStorage, sanitizerService), Sanitizer: sanitizerService, } } diff --git a/pkg/porter/reconcile_test.go b/pkg/porter/reconcile_test.go index 426315029..385a8a6b8 100644 --- a/pkg/porter/reconcile_test.go +++ b/pkg/porter/reconcile_test.go @@ -73,10 +73,9 @@ func TestPorter_IsInstallationInSync(t *testing.T) { i := storage.NewInstallation("", "mybuns") i.ParameterSets = []string{"myps"} i.Status.Installed = &now - run := storage.Run{ - // Use the default values from the bundle.json so that we don't trigger reconciliation - Parameters: storage.NewInternalParameterSet(i.Namespace, i.Name, storage.ValueStrategy("my-second-param", "override")), - } + run := i.NewRun(cnab.ActionUpgrade, bun) + // Use the default values from the bundle.json so that we don't trigger reconciliation + run.Parameters.Parameters = []secrets.SourceMap{storage.ValueStrategy("my-second-param", "override")} upgradeOpts := NewUpgradeOptions() upgradeOpts.bundleRef = &cnab.BundleReference{Definition: bun} require.NoError(t, p.applyActionOptionsToInstallation(ctx, upgradeOpts, &i)) @@ -115,9 +114,8 @@ func TestPorter_IsInstallationInSync(t *testing.T) { i := storage.NewInstallation("", "mybuns") i.Status.Installed = &now - run := storage.Run{ - Parameters: storage.NewInternalParameterSet(i.Namespace, i.Name, storage.ValueStrategy("my-second-param", "newvalue")), - } + run := i.NewRun(cnab.ActionUpgrade, bun) + run.Parameters.Parameters = []secrets.SourceMap{storage.ValueStrategy("my-second-param", "newvalue")} upgradeOpts := NewUpgradeOptions() upgradeOpts.bundleRef = &cnab.BundleReference{Definition: bun} require.NoError(t, p.applyActionOptionsToInstallation(ctx, upgradeOpts, &i)) @@ -137,11 +135,10 @@ func TestPorter_IsInstallationInSync(t *testing.T) { i := storage.NewInstallation("", "mybuns") i.Status.Installed = &now i.CredentialSets = []string{"newcreds"} - run := storage.Run{ - CredentialSets: []string{"oldcreds"}, - // Use the default values from the bundle.json so they don't trigger the reconciliation - Parameters: storage.NewInternalParameterSet(i.Namespace, i.Name, storage.ValueStrategy("my-second-param", "spring-music-demo")), - } + run := i.NewRun(cnab.ActionUpgrade, bun) + run.CredentialSets = []string{"oldcreds"} + // Use the default values from the bundle.json so they don't trigger the reconciliation + run.Parameters.Parameters = []secrets.SourceMap{storage.ValueStrategy("my-second-param", "spring-music-demo")} upgradeOpts := NewUpgradeOptions() upgradeOpts.bundleRef = &cnab.BundleReference{Definition: bun} require.NoError(t, p.applyActionOptionsToInstallation(ctx, upgradeOpts, &i)) diff --git a/pkg/porter/show_test.go b/pkg/porter/show_test.go index e6ce69c7e..568e6374a 100644 --- a/pkg/porter/show_test.go +++ b/pkg/porter/show_test.go @@ -113,16 +113,15 @@ func TestPorter_ShowInstallationWithBundle(t *testing.T) { storage.ValueStrategy("secretString", "foo"), ) - r.Parameters = i.NewInternalParameterSet( - []secrets.SourceMap{ - storage.ValueStrategy("logLevel", "3"), - storage.ValueStrategy("token", "top-secret"), - storage.ValueStrategy("secretString", "foo"), - }...) + params := []secrets.SourceMap{ + storage.ValueStrategy("logLevel", "3"), + storage.ValueStrategy("token", "top-secret"), + storage.ValueStrategy("secretString", "foo"), + } r.ParameterSets = []string{"dev-env"} r.ParameterOverrides.Parameters = p.SanitizeParameters(r.ParameterOverrides.Parameters, r.ID, bun) - r.Parameters.Parameters = p.SanitizeParameters(r.Parameters.Parameters, r.ID, bun) + r.Parameters.Parameters = p.SanitizeParameters(params, r.ID, bun) }) i.Parameters.Parameters = run.ParameterOverrides.Parameters diff --git a/pkg/porter/testdata/test-creds/mycreds.yaml b/pkg/porter/testdata/test-creds/mycreds.yaml new file mode 100644 index 000000000..5de2b6817 --- /dev/null +++ b/pkg/porter/testdata/test-creds/mycreds.yaml @@ -0,0 +1,8 @@ +name: mycreds +credentials: + - name: my-first-cred + source: + value: "sekret" + - name: my-second-cred + source: + value: "another sekret" \ No newline at end of file diff --git a/pkg/secrets/strategy.go b/pkg/secrets/strategy.go index 498328a48..cbefefa92 100644 --- a/pkg/secrets/strategy.go +++ b/pkg/secrets/strategy.go @@ -5,7 +5,6 @@ import ( "errors" "fmt" - "github.com/cnabio/cnab-go/valuesource" "gopkg.in/yaml.v3" ) @@ -27,22 +26,6 @@ func (s Set) Merge(s2 Set) error { return nil } -// IsValid determines if the provided key (designating a name of a parameter -// or credential) is included in the provided set -func (s Set) IsValid(key string) bool { - for name := range s { - if name == key { - return true - } - } - return false -} - -// ToCNAB converts this to a type accepted by the cnab-go runtime. -func (s Set) ToCNAB() valuesource.Set { - return valuesource.Set(s) -} - // SourceMap maps from a parameter or credential name to a source strategy for resolving its value. type SourceMap struct { // Name is the name of the parameter or credential. diff --git a/pkg/storage/credentialset.go b/pkg/storage/credentialset.go index 651fd5252..71f5f2588 100644 --- a/pkg/storage/credentialset.go +++ b/pkg/storage/credentialset.go @@ -2,6 +2,7 @@ package storage import ( "context" + "encoding/json" "fmt" "strings" "time" @@ -11,6 +12,7 @@ import ( "get.porter.sh/porter/pkg/secrets" "get.porter.sh/porter/pkg/tracing" "github.com/cnabio/cnab-go/bundle" + "github.com/cnabio/cnab-go/valuesource" "go.opentelemetry.io/otel/attribute" ) @@ -29,7 +31,7 @@ var _ Document = CredentialSet{} // handles accessing secrets. type CredentialSet struct { CredentialSetSpec `yaml:",inline"` - Status CredentialSetStatus `json:"status" yaml:"status" toml:"status"` + Status CredentialSetStatus `json:"status,omitempty" yaml:"status,omitempty" toml:"status,omitempty"` } // CredentialSetSpec represents the set of user-modifiable fields on a CredentialSet. @@ -50,16 +52,25 @@ type CredentialSetSpec struct { Labels map[string]string `json:"labels,omitempty" yaml:"labels,omitempty" toml:"labels,omitempty"` // Credentials is a list of credential resolution strategies. - Credentials secrets.StrategyList `json:"credentials" yaml:"credentials" toml:"credentials"` + Credentials secrets.StrategyList `json:"credentials,omitempty" yaml:"credentials,omitempty" toml:"credentials,omitempty"` } +// We implement a custom json marshal instead of using tags, so that we can omit zero-value timestamps +var _ json.Marshaler = CredentialSetStatus{} + // CredentialSetStatus contains additional status metadata that has been set by Porter. type CredentialSetStatus struct { // Created timestamp. - Created time.Time `json:"created" yaml:"created" toml:"created"` + Created time.Time `json:"created,omitempty" yaml:"created,omitempty" toml:"created,omitempty"` // Modified timestamp. - Modified time.Time `json:"modified" yaml:"modified" toml:"modified"` + Modified time.Time `json:"modified,omitempty" yaml:"modified,omitempty" toml:"modified,omitempty"` +} + +func NewInternalCredentialSet(creds ...secrets.SourceMap) CredentialSet { + return CredentialSet{ + CredentialSetSpec: CredentialSetSpec{Credentials: creds}, + } } // NewCredentialSet creates a new CredentialSet with the required fields initialized. @@ -82,6 +93,17 @@ func NewCredentialSet(namespace string, name string, creds ...secrets.SourceMap) return cs } +func (c CredentialSetStatus) MarshalJSON() ([]byte, error) { + raw := make(map[string]interface{}, 2) + if !c.Created.IsZero() { + raw["created"] = c.Created + } + if !c.Modified.IsZero() { + raw["modified"] = c.Modified + } + return json.Marshal(raw) +} + func (s CredentialSet) DefaultDocumentFilter() map[string]interface{} { return map[string]interface{}{"namespace": s.Namespace, "name": s.Name} } @@ -123,6 +145,26 @@ func (s CredentialSet) String() string { return fmt.Sprintf("%s/%s", s.Namespace, s.Name) } +// ToCNAB converts this to a type accepted by the cnab-go runtime. +func (s CredentialSet) ToCNAB() valuesource.Set { + values := make(valuesource.Set, len(s.Credentials)) + for _, cred := range s.Credentials { + values[cred.Name] = cred.ResolvedValue + } + return values +} + +// HasCredential determines if the specified credential is defined in the set. +func (s CredentialSet) HasCredential(name string) bool { + for _, cred := range s.Credentials { + if cred.Name == name { + return true + } + } + + return false +} + // Validate compares the given credentials with the spec. // // This will result in an error only when the following conditions are true: @@ -132,13 +174,13 @@ func (s CredentialSet) String() string { // // It is allowed for spec to specify both an env var and a file. In such case, if // the given set provides either, it will be considered valid. -func Validate(given secrets.Set, spec map[string]bundle.Credential, action string) error { +func (s CredentialSet) ValidateBundle(spec map[string]bundle.Credential, action string) error { for name, cred := range spec { if !cred.AppliesTo(action) { continue } - if !given.IsValid(name) && cred.Required { + if !s.HasCredential(name) && cred.Required { return fmt.Errorf("bundle requires credential for %s", name) } } diff --git a/pkg/storage/credentialset_test.go b/pkg/storage/credentialset_test.go index 76fa008b6..0a2f486d9 100644 --- a/pkg/storage/credentialset_test.go +++ b/pkg/storage/credentialset_test.go @@ -40,10 +40,12 @@ func TestValidate(t *testing.T) { spec := map[string]bundle.Credential{ "kubeconfig": {}, } - values := secrets.Set{ - "kubeconfig": "top secret creds", - } - err := Validate(values, spec, "install") + cs := CredentialSet{CredentialSetSpec: CredentialSetSpec{ + Credentials: []secrets.SourceMap{ + {Name: "kubeconfig", ResolvedValue: "top secret creds"}, + }}} + + err := cs.ValidateBundle(spec, "install") require.NoError(t, err, "expected Validate to pass because the credential was specified") }) @@ -51,8 +53,8 @@ func TestValidate(t *testing.T) { spec := map[string]bundle.Credential{ "kubeconfig": {ApplyTo: []string{"install"}, Required: false}, } - values := secrets.Set{} - err := Validate(values, spec, "install") + cs := CredentialSet{} + err := cs.ValidateBundle(spec, "install") require.NoError(t, err, "expected Validate to pass because the credential isn't required") }) @@ -60,8 +62,8 @@ func TestValidate(t *testing.T) { spec := map[string]bundle.Credential{ "kubeconfig": {ApplyTo: []string{"install"}, Required: true}, } - values := secrets.Set{} - err := Validate(values, spec, "custom") + cs := CredentialSet{} + err := cs.ValidateBundle(spec, "custom") require.NoError(t, err, "expected Validate to pass because the credential isn't applicable to the custom action") }) @@ -69,8 +71,8 @@ func TestValidate(t *testing.T) { spec := map[string]bundle.Credential{ "kubeconfig": {ApplyTo: []string{"install"}, Required: true}, } - values := secrets.Set{} - err := Validate(values, spec, "install") + cs := CredentialSet{} + err := cs.ValidateBundle(spec, "install") require.Error(t, err, "expected Validate to fail because the credential applies to the specified action and is required") assert.Contains(t, err.Error(), "bundle requires credential") }) diff --git a/pkg/storage/migrations/migration.go b/pkg/storage/migrations/migration.go index adb82aff9..e9fcdd65d 100644 --- a/pkg/storage/migrations/migration.go +++ b/pkg/storage/migrations/migration.go @@ -278,10 +278,11 @@ func (m *Migration) migrateClaim(ctx context.Context, inst *storage.Installation // Sanitize sensitive values on the source claim bun := cnab.ExtendedBundle{Bundle: run.Bundle} - run.Parameters.Parameters, err = m.sanitizer.CleanParameters(ctx, run.Parameters.Parameters, bun, run.ID) + cleanParams, err := m.sanitizer.CleanParameters(ctx, run.Parameters.Parameters, bun, run.ID) if err != nil { return span.Error(err) } + run.Parameters.Parameters = cleanParams // Find all results associated with the run resultIDs, err := m.listItems("results", run.ID) @@ -320,18 +321,19 @@ func convertClaimToRun(inst storage.Installation, data []byte) (storage.Run, err } dest := storage.Run{ - SchemaVersion: storage.DefaultInstallationSchemaVersion, - ID: src.ID, - Created: src.Created, - Namespace: inst.Namespace, - Installation: src.Installation, - Revision: src.Revision, - Action: src.Action, - Bundle: src.Bundle, - BundleReference: src.BundleReference, - BundleDigest: "", // We didn't track digest before v1 - Parameters: storage.NewInternalParameterSet(inst.Namespace, src.ID, params...), - Custom: src.Custom, + SchemaVersion: storage.DefaultInstallationSchemaVersion, + ID: src.ID, + Created: src.Created, + Namespace: inst.Namespace, + Installation: src.Installation, + Revision: src.Revision, + Action: src.Action, + Bundle: src.Bundle, + BundleReference: src.BundleReference, + BundleDigest: "", // We didn't track digest before v1 + Parameters: storage.NewInternalParameterSet(inst.Namespace, src.ID, params...), + Custom: src.Custom, + ParametersDigest: "", // Leave blank, and Porter will re-resolve later if needed. This is just a cached value to improve performance. } return dest, nil diff --git a/pkg/storage/parameterset.go b/pkg/storage/parameterset.go index 66f104c8c..4355d34b1 100644 --- a/pkg/storage/parameterset.go +++ b/pkg/storage/parameterset.go @@ -10,6 +10,7 @@ import ( "get.porter.sh/porter/pkg/schema" "get.porter.sh/porter/pkg/secrets" "get.porter.sh/porter/pkg/tracing" + "github.com/cnabio/cnab-go/bundle" "go.opentelemetry.io/otel/attribute" ) @@ -121,3 +122,36 @@ func (s *ParameterSet) Validate(ctx context.Context, strategy schema.CheckStrate func (s ParameterSet) String() string { return fmt.Sprintf("%s/%s", s.Namespace, s.Name) } + +// HasParameter determines if the specified parameter is defined in the set. +func (s ParameterSet) HasParameter(name string) bool { + for _, param := range s.Parameters { + if param.Name == name { + return true + } + } + + return false +} + +// ValidateBundle compares the given parameters with the spec in the bundle. +// +// This will result in an error only when the following conditions are true: +// - a parameter in the spec is not present in the given set +// - the parameters is required +// - the parameter applies to the specified action +// +// It is allowed for spec to specify both an env var and a file. In such case, if +// the given set provides either, it will be considered valid. +func (s ParameterSet) ValidateBundle(spec map[string]bundle.Parameter, action string) error { + for name, param := range spec { + if !param.AppliesTo(action) { + continue + } + + if !s.HasParameter(name) && param.Required { + return fmt.Errorf(`parameter "%s" is required`, name) + } + } + return nil +} diff --git a/pkg/storage/parameterset_test.go b/pkg/storage/parameterset_test.go index fa776cedb..9f2ed9fc1 100644 --- a/pkg/storage/parameterset_test.go +++ b/pkg/storage/parameterset_test.go @@ -9,6 +9,7 @@ import ( "get.porter.sh/porter/pkg/schema" "get.porter.sh/porter/pkg/secrets" "get.porter.sh/porter/tests" + "github.com/cnabio/cnab-go/bundle" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -111,3 +112,46 @@ func TestParameterSet_Validate_DefaultSchemaType(t *testing.T) { require.NoError(t, ps.Validate(context.Background(), schema.CheckStrategyExact)) assert.Equal(t, SchemaTypeParameterSet, ps.SchemaType) } + +func TestParameterSetValidateBundle(t *testing.T) { + t.Run("valid - parameter specified", func(t *testing.T) { + spec := map[string]bundle.Parameter{ + "kubeconfig": {}, + } + ps := ParameterSet{ParameterSetSpec: ParameterSetSpec{ + Parameters: []secrets.SourceMap{ + {Name: "kubeconfig", ResolvedValue: "top secret param"}, + }}} + + err := ps.ValidateBundle(spec, "install") + require.NoError(t, err, "expected Validate to pass because the parameter was specified") + }) + + t.Run("valid - parameter not required or specified", func(t *testing.T) { + spec := map[string]bundle.Parameter{ + "kubeconfig": {ApplyTo: []string{"install"}, Required: false}, + } + ps := ParameterSet{} + err := ps.ValidateBundle(spec, "install") + require.NoError(t, err, "expected Validate to pass because the parameter isn't required") + }) + + t.Run("valid - missing inapplicable parameter", func(t *testing.T) { + spec := map[string]bundle.Parameter{ + "kubeconfig": {ApplyTo: []string{"install"}, Required: true}, + } + ps := ParameterSet{} + err := ps.ValidateBundle(spec, "custom") + require.NoError(t, err, "expected Validate to pass because the parameter isn't applicable to the custom action") + }) + + t.Run("invalid - missing required parameter", func(t *testing.T) { + spec := map[string]bundle.Parameter{ + "kubeconfig": {ApplyTo: []string{"install"}, Required: true}, + } + ps := ParameterSet{} + err := ps.ValidateBundle(spec, "install") + require.Error(t, err, "expected Validate to fail because the parameter applies to the specified action and is required") + assert.Contains(t, err.Error(), `parameter "kubeconfig" is required`) + }) +} diff --git a/pkg/storage/run.go b/pkg/storage/run.go index c4aaf13aa..c373908fc 100644 --- a/pkg/storage/run.go +++ b/pkg/storage/run.go @@ -1,6 +1,7 @@ package storage import ( + "crypto/sha256" "encoding/json" "fmt" "time" @@ -13,7 +14,9 @@ var _ Document = Run{} var _ json.Marshaler = Run{} var _ json.Unmarshaler = &Run{} -// Run represents the execution of an installation's bundle. +// Run represents the execution of an installation's bundle. It contains both the +// instructions used by Porter to run the bundle, and additional status/audit +// fields so users can keep track of how the bundle was run. type Run struct { // SchemaVersion of the document. SchemaVersion cnab.SchemaVersion `json:"schemaVersion"` @@ -24,6 +27,10 @@ type Run struct { // Created timestamp of the Run. Created time.Time `json:"created"` + // Modified timestamp of the Run, set when we resolve run parameters just-in-time. + // A run can be created ahead of time as Pending and not have its parameters resolved until much later. + Modified time.Time `json:"modified"` + // Namespace of the installation. Namespace string `json:"namespace"` @@ -49,23 +56,47 @@ type Run struct { // ParameterOverrides are the key/value parameter overrides (taking precedence over // parameters specified in a parameter set) specified during the run. + // This is a status/audit field and is not used to resolve parameters for a Run. ParameterOverrides ParameterSet `json:"parameterOverrides,omitempty"` // CredentialSets is a list of the credential set names used during the run. + // This is a status/audit field and is not used to resolve credentials for a Run. CredentialSets []string `json:"credentialSets,omitempty"` // ParameterSets is the list of parameter set names used during the run. + // This is a status/audit field and is not used to resolve parameters for a Run. ParameterSets []string `json:"parameterSets,omitempty"` - // Parameters is the full set of parameters that's being used during the - // current run. - // This includes internal parameters, parameter sources, values from parameter sets, etc. - // Any sensitive data will be sannitized before saving to the database. + // Parameters is the full set of parameters that should be resolved just-in-time + // (JIT) before executing the bundle. This includes internal parameters, + // parameter sources, values from parameter sets, etc. These should be a "clean" + // set of parameters that have sensitive values persisted in secrets using the + // Sanitizer. + // After the parameters are resolved, this structure holds (but does not marshal) + // the resolved values, in addition to the mapping strategy. Parameters ParameterSet `json:"parameters,omitempty"` // Custom extension data applicable to a given runtime. // TODO(carolynvs): remove custom and populate it in ToCNAB Custom interface{} `json:"custom"` + + // ParametersDigest is a hash or digest of the final set of parameters, which allows us to + // quickly determine if the parameters have changed without requiring that they + // are re-resolved. The value should contain the hash type, e.g. sha256:abc123... + // This is a status/audit field and is not used to resolve parameters for a Run. + ParametersDigest string `json:"parametersDigest,omitempty"` + + // Credentials is the full set of credentials that should be resolved + // just-in-time (JIT) before executing the bundle. These should be a "clean" set + // of parameters that have sensitive values persisted in secrets using the + // Sanitizer. + Credentials CredentialSet `json:"credentials,omitempty"` + + // CredentialsDigest is a hash or digest of the final set of credentials, which allows us to + // quickly determine if the credentials have changed without requiring that they + // are re-resolved. The value should contain the hash type, e.g. sha256:abc123... + // This is a status/audit field and is not used to resolve credentials for a Run. + CredentialsDigest string `json:"credentialsDigest,omitempty"` } // rawRun is an alias for Run that does not have a json marshal functions defined, @@ -117,6 +148,7 @@ func NewRun(namespace string, installation string) Run { ID: cnab.NewULID(), Revision: cnab.NewULID(), Created: time.Now(), + Modified: time.Now(), Namespace: namespace, Installation: installation, Parameters: NewInternalParameterSet(namespace, installation), @@ -194,6 +226,36 @@ func (r Run) TypedParameterValues() map[string]interface{} { } +// SetParametersDigest records the hash of the resolved parameters, so we can +// quickly tell if the parameters between runs were different without +// re-resolving them. +func (r *Run) SetParametersDigest() error { + // Calculate a hash of the resolved parameters + paramB, err := json.Marshal(r.Parameters.Parameters) + if err != nil { + r.ParametersDigest = "" + return fmt.Errorf("error calculating the digest of the run parameters: %w", err) + } + + r.ParametersDigest = fmt.Sprintf("sha256:%x", sha256.Sum256(paramB)) + return nil +} + +// SetCredentialsDigest records the hash of the resolved credentials, so we can +// quickly tell if the parameters between runs were different without +// re-resolving them. +func (r *Run) SetCredentialsDigest() error { + // Calculate a hash of the resolved credentials + credB, err := json.Marshal(r.Credentials.Credentials) + if err != nil { + r.CredentialsDigest = "" + return fmt.Errorf("error calculating the digest of the run credentials: %w", err) + } + + r.CredentialsDigest = fmt.Sprintf("sha256:%x", sha256.Sum256(credB)) + return nil +} + // NewRun creates a result for the current Run. func (r Run) NewResult(status string) Result { result := NewResult() diff --git a/pkg/storage/run_test.go b/pkg/storage/run_test.go index 063bad9c4..902414cd2 100644 --- a/pkg/storage/run_test.go +++ b/pkg/storage/run_test.go @@ -171,13 +171,11 @@ func TestRun_TypedParameterValues(t *testing.T) { run := NewRun("dev", "mybuns") run.Bundle = bun - run.Parameters = NewParameterSet(run.Namespace, run.Bundle.Name) - params := []secrets.SourceMap{ + run.Parameters = NewParameterSet(run.Namespace, run.Bundle.Name, ValueStrategy("baz", "baz-test"), ValueStrategy("name", "porter-test"), ValueStrategy("porter-state", ""), - {Name: "foo", Source: secrets.Source{Strategy: secrets.SourceSecret, Hint: "runID"}, ResolvedValue: "5"}, - } + secrets.SourceMap{Name: "foo", Source: secrets.Source{Strategy: secrets.SourceSecret, Hint: "runID"}, ResolvedValue: "5"}) expected := map[string]interface{}{ "baz": "baz-test", @@ -186,9 +184,8 @@ func TestRun_TypedParameterValues(t *testing.T) { "foo": 5, } - run.Parameters.Parameters = params typed := run.TypedParameterValues() - require.Equal(t, len(params), len(typed)) + require.Equal(t, len(run.Parameters.Parameters), len(typed)) require.Equal(t, len(expected), len(typed)) for name, value := range typed { diff --git a/pkg/storage/testdata/marshaled_run.json b/pkg/storage/testdata/marshaled_run.json index 5eeb58150..8988696e5 100644 --- a/pkg/storage/testdata/marshaled_run.json +++ b/pkg/storage/testdata/marshaled_run.json @@ -1 +1 @@ -{"schemaVersion":"","_id":"foo","created":"0001-01-01T00:00:00Z","namespace":"","installation":"","revision":"","action":"","bundleReference":"","bundleDigest":"","parameterOverrides":{"schemaVersion":"","namespace":"","name":"","parameters":null,"status":{"created":"0001-01-01T00:00:00Z","modified":"0001-01-01T00:00:00Z"}},"parameters":{"schemaVersion":"","namespace":"","name":"","parameters":null,"status":{"created":"0001-01-01T00:00:00Z","modified":"0001-01-01T00:00:00Z"}},"custom":null,"bundle":"{\"actions\":{\"logs\":{},\"test\":{\"modifies\":true}},\"description\":\"this is my bundle\",\"invocationImages\":[],\"name\":\"mybun\",\"schemaVersion\":\"schemaVersion\",\"version\":\"v0.1.0\"}"} \ No newline at end of file +{"schemaVersion":"","_id":"foo","created":"0001-01-01T00:00:00Z","modified":"0001-01-01T00:00:00Z","namespace":"","installation":"","revision":"","action":"","bundleReference":"","bundleDigest":"","parameterOverrides":{"schemaVersion":"","namespace":"","name":"","parameters":null,"status":{"created":"0001-01-01T00:00:00Z","modified":"0001-01-01T00:00:00Z"}},"parameters":{"schemaVersion":"","namespace":"","name":"","parameters":null,"status":{"created":"0001-01-01T00:00:00Z","modified":"0001-01-01T00:00:00Z"}},"custom":null,"credentials":{"schemaVersion":"","namespace":"","name":"","status":{}},"bundle":"{\"actions\":{\"logs\":{},\"test\":{\"modifies\":true}},\"description\":\"this is my bundle\",\"invocationImages\":[],\"name\":\"mybun\",\"schemaVersion\":\"schemaVersion\",\"version\":\"v0.1.0\"}"} \ No newline at end of file