Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor shared bundle action options #2245

Merged
merged 2 commits into from
Jul 25, 2022
Merged

Refactor shared bundle action options #2245

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d618364
Refactor shared bundle action options
carolynvs Jul 13, 2022
5c30a02
Review feedback
carolynvs Jul 21, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 15 additions & 20 deletions cmd/porter/bundle.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (

"get.porter.sh/porter/pkg/porter"
"github.com/spf13/cobra"
"github.com/spf13/pflag"
)

func buildBundleCommands(p *porter.Porter) *cobra.Command {
Expand Down Expand Up @@ -172,8 +173,6 @@ The docker driver runs the bundle container using the local Docker host. To use
}

f := cmd.Flags()
f.BoolVar(&opts.AllowDockerHostAccess, "allow-docker-host-access", false,
"Controls if the bundle should have access to the host's Docker daemon with elevated privileges. See https://getporter.org/configuration/#allow-docker-host-access for the full implications of this flag.")
f.StringVarP(&opts.File, "file", "f", "",
"Path to the porter manifest file. Defaults to the bundle in the current directory.")
f.StringVar(&opts.CNABFile, "cnab-file", "",
Expand All @@ -190,9 +189,7 @@ The docker driver runs the bundle container using the local Docker host. To use
"Create the installation in the specified namespace. Defaults to the global namespace.")
f.StringSliceVarP(&opts.Labels, "label", "l", nil,
"Associate the specified labels with the installation. May be specified multiple times.")
f.BoolVar(&opts.NoLogs, "no-logs", false,
"Do not persist the bundle execution logs")
addBundlePullFlags(f, &opts.BundlePullOptions)
addBundleActionFlags(f, opts)

// Allow configuring the --driver flag with runtime-driver, to avoid conflicts with other commands
cmd.Flag("driver").Annotations = map[string][]string{
Expand Down Expand Up @@ -235,8 +232,6 @@ The docker driver runs the bundle container using the local Docker host. To use
}

f := cmd.Flags()
f.BoolVar(&opts.AllowDockerHostAccess, "allow-docker-host-access", false,
"Controls if the bundle should have access to the host's Docker daemon with elevated privileges. See https://getporter.org/configuration/#allow-docker-host-access for the full implications of this flag.")
f.StringVarP(&opts.File, "file", "f", "",
"Path to the porter manifest file. Defaults to the bundle in the current directory.")
f.StringVar(&opts.CNABFile, "cnab-file", "",
Expand All @@ -253,9 +248,7 @@ The docker driver runs the bundle container using the local Docker host. To use
"Namespace of the specified installation. Defaults to the global namespace.")
f.StringVar(&opts.Version, "version", "",
"Version to which the installation should be upgraded. This represents the version of the bundle, which assumes the convention of setting the bundle tag to its version.")
f.BoolVar(&opts.NoLogs, "no-logs", false,
"Do not persist the bundle execution logs")
addBundlePullFlags(f, &opts.BundlePullOptions)
addBundleActionFlags(f, opts)

// Allow configuring the --driver flag with runtime-driver, to avoid conflicts with other commands
cmd.Flag("driver").Annotations = map[string][]string{
Expand Down Expand Up @@ -298,8 +291,6 @@ The docker driver runs the bundle container using the local Docker host. To use
}

f := cmd.Flags()
f.BoolVar(&opts.AllowDockerHostAccess, "allow-docker-host-access", false,
"Controls if the bundle should have access to the host's Docker daemon with elevated privileges. See https://getporter.org/configuration/#allow-docker-host-access for the full implications of this flag.")
f.StringVar(&opts.Action, "action", "",
"Custom action name to invoke.")
f.StringVarP(&opts.File, "file", "f", "",
Expand All @@ -316,9 +307,7 @@ The docker driver runs the bundle container using the local Docker host. To use
"Specify a driver to use. Allowed values: docker, debug")
f.StringVarP(&opts.Namespace, "namespace", "n", "",
"Namespace of the specified installation. Defaults to the global namespace.")
f.BoolVar(&opts.NoLogs, "no-logs", false,
"Do not persist the bundle execution logs")
addBundlePullFlags(f, &opts.BundlePullOptions)
addBundleActionFlags(f, opts)

// Allow configuring the --driver flag with runtime-driver, to avoid conflicts with other commands
cmd.Flag("driver").Annotations = map[string][]string{
Expand Down Expand Up @@ -363,8 +352,6 @@ The docker driver runs the bundle container using the local Docker host. To use
}

f := cmd.Flags()
f.BoolVar(&opts.AllowDockerHostAccess, "allow-docker-host-access", false,
"Controls if the bundle should have access to the host's Docker daemon with elevated privileges. See https://getporter.org/configuration/#allow-docker-host-access for the full implications of this flag.")
f.StringVarP(&opts.File, "file", "f", "",
"Path to the porter manifest file. Defaults to the bundle in the current directory. Optional unless a newer version of the bundle should be used to uninstall the bundle.")
f.StringVar(&opts.CNABFile, "cnab-file", "",
Expand All @@ -383,9 +370,7 @@ The docker driver runs the bundle container using the local Docker host. To use
"UNSAFE. Delete all records associated with the installation, even if uninstall fails. This is intended for cleaning up test data and is not recommended for production environments.")
f.StringVarP(&opts.Namespace, "namespace", "n", "",
"Namespace of the specified installation. Defaults to the global namespace.")
f.BoolVar(&opts.NoLogs, "no-logs", false,
"Do not persist the bundle execution logs")
addBundlePullFlags(f, &opts.BundlePullOptions)
addBundleActionFlags(f, opts)

// Allow configuring the --driver flag with runtime-driver, to avoid conflicts with other commands
cmd.Flag("driver").Annotations = map[string][]string{
Expand Down Expand Up @@ -454,3 +439,13 @@ func buildBundleArchiveCommand(p *porter.Porter) *cobra.Command {

return &cmd
}

// Add flags for command that execute a bundle (install, upgrade, invoke and uninstall)
func addBundleActionFlags(f *pflag.FlagSet, actionOpts porter.BundleAction) {
opts := actionOpts.GetOptions()
addBundlePullFlags(f, &opts.BundlePullOptions)
f.BoolVar(&opts.AllowDockerHostAccess, "allow-docker-host-access", false,
"Controls if the bundle should have access to the host's Docker daemon with elevated privileges. See https://getporter.org/configuration/#allow-docker-host-access for the full implications of this flag.")
f.BoolVar(&opts.NoLogs, "no-logs", false,
"Do not persist the bundle execution logs")
}
7 changes: 7 additions & 0 deletions pkg/config/datastore.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,12 @@ const (
// BuildDriverBuildkit is the configuration value for specifying BuildKit as
// the build driver.
BuildDriverBuildkit = "buildkit"

// RuntimeDriverDocker specifies that the invocation image should be executed on docker.
RuntimeDriverDocker = "docker"

// RuntimeDriverKubernetes specifies that the invocation image should be executed on kubernetes.
RuntimeDriverKubernetes = "kubernetes"
)

// Data is the data stored in PORTER_HOME/porter.toml|yaml|json.
Expand Down Expand Up @@ -70,6 +76,7 @@ type Data struct {
func DefaultDataStore() Data {
return Data{
BuildDriver: BuildDriverBuildkit,
RuntimeDriver: RuntimeDriverDocker,
DefaultStoragePlugin: "mongodb-docker",
DefaultSecretsPlugin: "host",
Logs: LogConfig{Level: "info"},
Expand Down
6 changes: 3 additions & 3 deletions pkg/porter/archive.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import (

// ArchiveOptions defines the valid options for performing an archive operation
type ArchiveOptions struct {
BundleActionOptions
BundleReferenceOptions
ArchiveFile string
}

Expand All @@ -42,7 +42,7 @@ func (o *ArchiveOptions) Validate(ctx context.Context, args []string, p *Porter)
if o.Reference == "" {
return errors.New("must provide a value for --reference of the form REGISTRY/bundle:tag")
}
return o.BundleActionOptions.Validate(ctx, args, p)
return o.BundleReferenceOptions.Validate(ctx, args, p)
}

// Archive is a composite function that generates a CNAB thick bundle. It will pull the invocation image, and
Expand All @@ -57,7 +57,7 @@ func (p *Porter) Archive(ctx context.Context, opts ArchiveOptions) error {
return log.Error(fmt.Errorf("parent directory %q does not exist", dir))
}

bundleRef, err := p.resolveBundleReference(ctx, &opts.BundleActionOptions)
bundleRef, err := p.resolveBundleReference(ctx, &opts.BundleReferenceOptions)
if err != nil {
return log.Error(err)
}
Expand Down
154 changes: 4 additions & 150 deletions pkg/porter/cnab.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,9 @@ import (
"path/filepath"

"get.porter.sh/porter/pkg/build"
"get.porter.sh/porter/pkg/cnab"
"get.porter.sh/porter/pkg/cnab/drivers"
cnabprovider "get.porter.sh/porter/pkg/cnab/provider"
"get.porter.sh/porter/pkg/config"
"get.porter.sh/porter/pkg/portercontext"
"get.porter.sh/porter/pkg/secrets"
"get.porter.sh/porter/pkg/storage"
)

const (
Expand Down Expand Up @@ -87,42 +83,21 @@ func (o *bundleFileOptions) Validate(cxt *portercontext.Context) error {
return nil
}

// sharedOptions are common options that apply to multiple CNAB actions.
type sharedOptions struct {
// installationOptions are common options that apply to commands that use an installation
type installationOptions struct {
bundleFileOptions

// Namespace of the installation.
Namespace string

// Name of the installation. Defaults to the name of the bundle.
Name string

// Params is the unparsed list of NAME=VALUE parameters set on the command line.
Params []string

// ParameterSets is a list of parameter sets containing parameter sources
ParameterSets []string

// CredentialIdentifiers is a list of credential names or paths to make available to the bundle.
CredentialIdentifiers []string

// Driver is the CNAB-compliant driver used to run bundle actions.
Driver string

// parsedParams is the parsed set of parameters from Params.
parsedParams map[string]string

// parsedParamSets is the parsed set of parameter from ParameterSets
parsedParamSets map[string]string

// combinedParameters is parsedParams merged on top of parsedParamSets.
combinedParameters map[string]string
}

// Validate prepares for an action and validates the options.
// For example, relative paths are converted to full paths and then checked that
// they exist and are accessible.
func (o *sharedOptions) Validate(ctx context.Context, args []string, p *Porter) error {
func (o *installationOptions) Validate(ctx context.Context, args []string, p *Porter) error {
err := o.validateInstallationName(args)
if err != nil {
return err
Expand All @@ -138,24 +113,11 @@ func (o *sharedOptions) Validate(ctx context.Context, args []string, p *Porter)
return err
}

// Only validate the syntax of the --param flags
// We will validate the parameter sets later once we have the bundle loaded.
err = o.parseParams()
if err != nil {
return err
}

o.defaultDriver()
err = o.validateDriver(p.Context)
if err != nil {
return err
}

return nil
}

// validateInstallationName grabs the installation name from the first positional argument.
func (o *sharedOptions) validateInstallationName(args []string) error {
func (o *installationOptions) validateInstallationName(args []string) error {
if len(args) == 1 {
o.Name = args[0]
} else if len(args) > 1 {
Expand Down Expand Up @@ -242,111 +204,3 @@ func (o *bundleFileOptions) validateCNABFile(cxt *portercontext.Context) error {

return nil
}

// LoadParameters validates and resolves the parameters and sets. It must be
// called after porter has loaded the bundle definition.
func (o *sharedOptions) LoadParameters(ctx context.Context, p *Porter, bun cnab.ExtendedBundle) error {
// This is called in multiple code paths, so exit early if
// we have already loaded the parameters into combinedParameters
if o.combinedParameters != nil {
return nil
}

err := o.parseParams()
if err != nil {
return err
}

err = o.parseParamSets(ctx, p, bun)
if err != nil {
return err
}

o.combinedParameters = o.combineParameters(p.Context)

return nil
}

// parsedParams parses the variable assignments in Params.
func (o *sharedOptions) parseParams() error {
p, err := storage.ParseVariableAssignments(o.Params)
if err != nil {
return err
}

o.parsedParams = p
return nil
}

func (o *sharedOptions) populateInternalParameterSet(ctx context.Context, p *Porter, bun cnab.ExtendedBundle, i *storage.Installation) error {
strategies := make([]secrets.Strategy, 0, len(o.parsedParams))
for name, value := range o.parsedParams {
strategies = append(strategies, storage.ValueStrategy(name, value))
}

strategies, err := p.Sanitizer.CleanParameters(ctx, strategies, bun, i.ID)
if err != nil {
return err
}

if len(strategies) == 0 {
// if no override is specified, clear out the old parameters on the
// installation record
i.Parameters.Parameters = nil
return nil
}

i.Parameters = i.NewInternalParameterSet(strategies...)

return nil
}

// parseParamSets parses the variable assignments in ParameterSets.
func (o *sharedOptions) parseParamSets(ctx context.Context, p *Porter, bun cnab.ExtendedBundle) error {
if len(o.ParameterSets) > 0 {
parsed, err := p.loadParameterSets(ctx, bun, o.Namespace, o.ParameterSets)
if err != nil {
return fmt.Errorf("unable to process provided parameter sets: %w", err)
}
o.parsedParamSets = parsed
}
return nil
}

// Combine the parameters into a single map
// The params set on the command line take precedence over the params set in
// parameter set files
// Anything set multiple times, is decided by "last one set wins"
func (o *sharedOptions) combineParameters(c *portercontext.Context) map[string]string {
final := make(map[string]string)

for k, v := range o.parsedParamSets {
final[k] = v
}

for k, v := range o.parsedParams {
final[k] = v
}

//
// Default the porter-debug param to --debug
//
if c.Debug {
final["porter-debug"] = "true"
}

return final
}

// defaultDriver supplies the default driver if none is specified
func (o *sharedOptions) defaultDriver() {
if o.Driver == "" {
o.Driver = DefaultDriver
}
}

// validateDriver validates that the provided driver is supported by Porter
func (o *sharedOptions) validateDriver(cxt *portercontext.Context) error {
_, err := drivers.LookupDriver(cxt, o.Driver)
return err
}
Loading