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.

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

Directory Param feature #2251

Open
wants to merge 13 commits into
base: release/v1
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ require (
github.com/osteele/liquid v1.3.0
github.com/pelletier/go-toml v1.9.4
github.com/pivotal/image-relocation v0.0.0-20191111101224-e94aff6df06c
github.com/pkg/errors v0.9.1
github.com/spf13/afero v1.5.1
github.com/spf13/cobra v1.2.1
github.com/spf13/pflag v1.0.5
Expand Down Expand Up @@ -173,7 +174,6 @@ require (
github.com/opencontainers/runc v1.1.2 // indirect
github.com/osteele/tuesday v1.0.3 // indirect
github.com/pierrec/lz4/v4 v4.0.3 // indirect
github.com/pkg/errors v0.9.1 // indirect
github.com/pmezard/go-difflib v1.0.0 // indirect
github.com/prometheus/client_golang v1.12.1 // indirect
github.com/prometheus/client_model v0.2.0 // indirect
Expand Down
Binary file added go1.18.3.linux-amd64.tar.gz
Binary file not shown.
120 changes: 107 additions & 13 deletions pkg/cnab/config-adapter/adapter.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package configadapter
import (
"context"
"fmt"
"os"
"path"
"strings"

Expand Down Expand Up @@ -80,7 +81,6 @@ func (c *ManifestConverter) ToBundle(ctx context.Context) (cnab.ExtendedBundle,
}
b.Custom = custom
b.RequiredExtensions = c.generateRequiredExtensions(b)

b.Custom[config.CustomPorterKey] = stamp

return b, nil
Expand Down Expand Up @@ -203,8 +203,11 @@ func (c *ManifestConverter) generateBundleParameters(ctx context.Context, defs *
}

if param.Type == nil {
// Default to a file type if the param is stored in a file
if param.Destination.Path != "" {
// Default to a directory type if the param is a directory
if param.Destination.Path != "" && strings.HasSuffix(param.Destination.Path, string(os.PathSeparator)) {
param.Type = "directory"
} else if param.Destination.Path != "" {
// If the path could refer to a file assume that it does unless specified explicity
param.Type = "file"
} else {
// Assume it's a string otherwise
Expand Down Expand Up @@ -298,12 +301,8 @@ func (c *ManifestConverter) addDefinition(name string, kind string, def definiti
defName = name + "-" + kind
}

// file is a porter specific type, swap it out for something CNAB understands
if def.Type == "file" {
def.Type = "string"
def.ContentEncoding = "base64"
}

// Type may be a porter specific type, swap it out for something CNAB understands
MakeCNABCompatible(&def)
(*defs)[defName] = &def

return defName
Expand Down Expand Up @@ -467,16 +466,31 @@ func (c *ManifestConverter) generateParameterSources(b *cnab.ExtendedBundle) cna
// 3. directly when they use `source` on a parameter

// Directly wired outputs to parameters
for _, p := range c.Manifest.Parameters {
// Skip parameters that aren't set from an output
if p.Source.Output == "" {
for k, p := range c.Manifest.Parameters {
// Skip parameters that aren't set from an output or from a directory source
if (!p.Source.IsDirSource()) && p.Source.Output == "" {
continue
}

var pso cnab.ParameterSource
if p.Source.Dependency == "" {
if p.Source.IsDirSource() {
// If it's a directory handle it accordingly
defName := fmt.Sprintf("%s-parameter", p.Name)
pso = c.generateDirectoryParameterSource(p.Source.Mount, p.Name, p.Destination.Path)
def := c.generateDirectoryParameterSchema(*b, defName)
// Make sure that the destination is changed to an env var instead of a path
// Otherwise cnab will attempt to place the path into the container which will fail
if pb, ok := b.Parameters[k]; ok {
c.sanitizeDirParameters(pb.Destination, k)
b.Parameters[k] = pb
}
b.Definitions[defName] = &def

} else if p.Source.Dependency == "" {
// If it's not a directory and it doesn't have a dependency, it's a standard output
pso = c.generateOutputParameterSource(p.Source.Output)
} else {
// Otherwise it must be a dependency
ref := manifest.DependencyOutputReference{
Dependency: p.Source.Dependency,
Output: p.Source.Output,
Expand Down Expand Up @@ -519,6 +533,28 @@ func (c *ManifestConverter) generateParameterSources(b *cnab.ExtendedBundle) cna
return ps
}

func (c *ManifestConverter) generateDirectoryParameterSchema(b cnab.ExtendedBundle, name string) definition.Schema {
var def definition.Schema
pdef, ok := b.Definitions[name]
if ok {
MakeCNABCompatible(b.Definitions[name])
def = *pdef
} else {
def = definition.Schema{}
def.Type = "directory"
MakeCNABCompatible(&def)
}
def.ID = "https://porter.sh/generated-bundle/#porter-parameter-source-definition"
return def
}

// Remove the path value from directory parameters so they aren't assumed to be files
// By the cnab.io package. Apply the destination to an env var "directory-parameters.[name]"
func(c *ManifestConverter) sanitizeDirParameters(destination *bundle.Location, name string) {
destination.Path = ""
destination.EnvironmentVariable = cnab.DirectoryExtensionShortHand + "." + name
}

// generateOutputWiringParameter creates an internal parameter used only by porter, it won't be visible to the user.
// The parameter exists solely so that Porter can inject an output back into the bundle, using a parameter source.
// The parameter's definition is a copy of the output's definition, with the ID set so we know that it was generated by porter.
Expand Down Expand Up @@ -580,6 +616,30 @@ func (c *ManifestConverter) generateOutputParameterSource(outputName string) cna
}
}

// Pass the inferred info from the parameter to the parameter source
func (c *ManifestConverter) generateDirectoryParameterSource(source interface{}, name string, target string) cnab.ParameterSource {
switch source.(type) {
case cnab.MountParameterSourceDefn:
return c.generateMountParameterSource(source.(cnab.MountParameterSourceDefn), name, target)
default:
return cnab.ParameterSource{}
}
}

// generateMountParameterSource builds a parameter source that connects a parameter to a mount.
func (c *ManifestConverter) generateMountParameterSource(mount cnab.MountParameterSourceDefn, name string, target string) cnab.ParameterSource {
return cnab.ParameterSource{
Priority: []string{cnab.ParameterSourceTypeMount},
Sources: map[string]cnab.ParameterSourceDefinition{
cnab.ParameterSourceTypeMount: func() cnab.MountParameterSourceDefn {
mount.Name = name
mount.Target = target
return mount
}(),
},
}
}

// generateDependencyOutputParameterSource builds a parameter source that connects a dependency output to a parameter.
func (c *ManifestConverter) generateDependencyOutputParameterSource(ref manifest.DependencyOutputReference) cnab.ParameterSource {
return cnab.ParameterSource{
Expand Down Expand Up @@ -630,6 +690,13 @@ func (c *ManifestConverter) generateCustomExtensions(b *cnab.ExtendedBundle) (ma
customExtensions[cnab.ParameterSourcesExtensionKey] = ps
}

// Add the directory extension
if dirs, err := c.generateDirectoryExtension(ps); err == nil && len(dirs) > 0 {
customExtensions[cnab.DirectoryParameterExtensionKey] = dirs
} else if err != nil {
fmt.Fprintln(os.Stderr, err.Error())
}

// Add entries for user-specified required extensions, like docker
for _, ext := range c.Manifest.Required {
customExtensions[lookupExtensionKey(ext.Name)] = ext.Config
Expand All @@ -638,6 +705,29 @@ func (c *ManifestConverter) generateCustomExtensions(b *cnab.ExtendedBundle) (ma
return customExtensions, nil
}

func (c *ManifestConverter) generateDirectoryExtension(ps cnab.ParameterSources) (map[string]cnab.DirectoryDetails, error) {
dirs := make(map[string]cnab.DirectoryDetails, 0)
for name, param := range ps {
for _, src := range param.Sources {
switch src.(type) {
case cnab.MountParameterSourceDefn:
dirs[name] = cnab.DirectoryDetails{
DirectorySources: cnab.DirectorySources{
Mount: src.(cnab.MountParameterSourceDefn),
},
DirectoryParameterDefinition: c.Manifest.Parameters[name].DirectoryParameterDefinition,
Kind: cnab.ParameterSourceTypeMount,
}
break
default:
continue
}
}
}

return dirs, nil
}

func (c *ManifestConverter) generateRequiredExtensions(b cnab.ExtendedBundle) []string {
requiredExtensions := []string{cnab.FileParameterExtensionKey}

Expand All @@ -651,6 +741,10 @@ func (c *ManifestConverter) generateRequiredExtensions(b cnab.ExtendedBundle) []
requiredExtensions = append(requiredExtensions, cnab.ParameterSourcesExtensionKey)
}

if b.HasDirectoryParameters() {
requiredExtensions = append(requiredExtensions, cnab.DirectoryParameterExtensionKey)
}

// Add all under required section of manifest
for _, ext := range c.Manifest.Required {
requiredExtensions = append(requiredExtensions, lookupExtensionKey(ext.Name))
Expand Down
17 changes: 17 additions & 0 deletions pkg/cnab/config-adapter/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"get.porter.sh/porter/pkg/cnab"
"get.porter.sh/porter/pkg/config"
"get.porter.sh/porter/pkg/manifest"
"github.com/cnabio/cnab-go/bundle/definition"
)

// ConvertToTestBundle is suitable for taking a test manifest (porter.yaml)
Expand All @@ -15,3 +16,19 @@ func ConvertToTestBundle(ctx context.Context, cfg *config.Config, manifest *mani
converter := NewManifestConverter(cfg, manifest, nil, nil)
return converter.ToBundle(ctx)
}

// MakeCNABCompatible receives a schema with possible porter specific parameters
// and converts those parameters to CNAB compatible versions.
// Returns true if values were replaced and false otherwise.
func MakeCNABCompatible(schema *definition.Schema) bool {
if v, ok := schema.Type.(string); ok {
if c, ok := config.PorterParamMap[v]; ok {
schema.Type = c.Type
schema.ContentEncoding = c.Encoding
schema.Comment = c.Comment
return ok
}
}

return false
}
104 changes: 104 additions & 0 deletions pkg/cnab/directory_parameter.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
package cnab

import (
"encoding/json"

"github.com/cnabio/cnab-go/bundle/definition"
"github.com/docker/docker/api/types/mount"
"github.com/pkg/errors"
)

const (
DirectoryExtensionShortHand = "directory-parameter"
DirectoryParameterExtensionKey = PorterExtensionsPrefix + DirectoryExtensionShortHand
)

// DirectoryParameterDefinition represents those parameter options
// That apply exclusively to the directory parameter type
type DirectoryParameterDefinition struct {
Writeable bool `yaml:"writeable,omitempty"`
// UID and GID should be ints, however 0 is the default value for int type
// But is also a realistic value for UID/GID thus we need to make the type interface
// To detect the case that the values weren't set
GID interface{} `yaml:"gid,omitempty" json:"gid,omitempty"`
UID interface{} `yaml:"uid,omitempty" json:"uid,omitempty"`
}

// MountParameterSource represents a parameter using a docker mount
// As a its source with the provided options
type MountParameterSourceDefn struct {
mount.Mount `yaml:",inline"`
Copy link
Member

@carolynvs carolynvs Jul 25, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's talk more about this right here. The goal is that the bundle doesn't know or care how the directory got there. Just that it has the access it needs.

Ideally the mount options should all be a concern of just the porter client and how we execute the cnab bundle as a docker image. I may not have been clear in my original comment, but what I was suggesting was that the user could specify the mount options using a docker mount string, either as a --param flag, or via a parameter set:

# this is a parameter set file, not the porter.yaml
name: mybundle-params
parameters:
- name: nginx-config
  source:
    # could specify whatever the docker --mount flag supports
    # target isn't required since Porter can determine that info from the parameter definition
    mount: type=bind,source=/home/myuser/myconfig,readonly

Then when the docker driver is used, it will use both the docker extension information in addition to the mount string from the parameter, to mount the directory into the container.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay I can use a string. My idea with using a larger object was so that we didn't have to parse the string, but it should otherwise simplify things a bit so I'll make the change.

Name string `json:"name,omitempty" yaml:"name,omitempty"`
}

// DirectorySources represents the sources available to the directory parameter type
// Currently only mount has been specified, but this could change in the future
type DirectorySources struct {
Mount MountParameterSourceDefn `yaml:"mount,omitempty" json:"mount,omitempty"`
}
type DirectoryDetails struct {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't tried this but here's what I think will fit better with how the CNAB spec and Porter works:

  • DirectoryDetails should have Name defined as well, or use a map[string]DirectoryDetails when storing the extension data and then key off of parameter name.
  • DirectoryDefaults should not have DirectorySources or Kind.
  • Do not create a new parameter source of type mount, I think we can get it to work without it and sources should be used for things that the bundle author knows about up-front (like using the output of a bundle as the parameter value), and bundle authors shouldn't be involved with whether or not a directory was injected using a docker mount, or through some other trick (like unpacking a gzip file into the specified directory).

DirectorySources
DirectoryParameterDefinition
Kind string `json:"kind,omitempty"`
}

// DirectoryParameterExtension indicates that Directory support is required
var DirectoryParameterExtension = RequiredExtension{
Shorthand: DirectoryExtensionShortHand,
Key: DirectoryParameterExtensionKey,
Reader: DirectoryParameterReader,
}

// SupportsDirectoryParameters returns true if the bundle supports the
// Directory parameter extension
func (b ExtendedBundle) SupportsDirectoryParameters() bool {
return b.SupportsExtension(DirectoryParameterExtensionKey)
}

// IsDirType determines if the parameter/credential is of type "directory".
func (b ExtendedBundle) IsDirType(def *definition.Schema) bool {
return b.SupportsDirectoryParameters() && def.Type == "string" && def.Comment == DirectoryParameterExtensionKey
}

// DirectoryParameterReader is a Reader for the DirectoryParameterExtension.
// The extension maintains the list of directory parameters in the bundle
func DirectoryParameterReader(b ExtendedBundle) (interface{}, error) {
return b.DirectoryParameterReader()
}

// DirectoryParameterReader is a Reader for the DirectoryParameterExtension.
// This method generates the list of directory parameter names in the bundle.
// The Directory Parameter extension maintains the list of directory parameters in the bundle
func (b ExtendedBundle) DirectoryParameterReader() (interface{}, error) {
bytes, err := json.Marshal(b.Custom[DirectoryParameterExtensionKey])
if err != nil {
return nil, errors.Wrapf(err, "Failed to marshal custom extension %s", DirectoryParameterExtensionKey)
}
var dd map[string]DirectoryDetails
if err = errors.Wrapf(json.Unmarshal(bytes, &dd), "Failed to unmarshal custom extension %s %s", DirectoryParameterExtensionKey, string(bytes)); err != nil {
return nil, err
}
dirs := make([]DirectoryDetails, len(dd))
i := 0
for _, dir := range dd {
dirs[i] = dir
i++
}
return dirs, nil
}

// DirectoryParameterSupport checks if the Directory parameter extension is present
func (e ProcessedExtensions) DirectoryParameterSupport() bool {
_, extensionRequired := e[DirectoryParameterExtensionKey]
return extensionRequired
}

// IDToInt converts an interface to an integer. If the id is coercable to an int, returns the value
// Otherwise returns -1
func IDToInt(id interface{}) int {
if i, ok := id.(int); ok {
return i
}

return -1
}
2 changes: 2 additions & 0 deletions pkg/cnab/extended_bundle.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,8 @@ func (b ExtendedBundle) GetParameterType(def *definition.Schema) string {
return fmt.Sprintf("%v", def.Type)
}



// IsFileType determines if the parameter/credential is of type "file".
func (b ExtendedBundle) IsFileType(def *definition.Schema) bool {
return b.SupportsFileParameters() &&
Expand Down
4 changes: 2 additions & 2 deletions pkg/cnab/file_parameter.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,13 @@ var FileParameterExtension = RequiredExtension{
Reader: FileParameterReader,
}


// FileParameterReader is a Reader for the FileParameterExtension.
// The extension does not have any data, its presence indicates that
// parameters of type "file" should be supported by the tooling.
func FileParameterReader(b ExtendedBundle) (interface{}, error) {
return b.FileParameterReader()
}

// FileParameterReader is a Reader for the FileParameterExtension.
// The extension does not have any data, its presence indicates that
// parameters of type "file" should be supported by the tooling.
Expand All @@ -45,4 +45,4 @@ func (b ExtendedBundle) SupportsFileParameters() bool {
func (e ProcessedExtensions) FileParameterSupport() bool {
_, extensionRequired := e[FileParameterExtensionKey]
return extensionRequired
}
}
Loading