Skip to content

Commit

Permalink
address todos
Browse files Browse the repository at this point in the history
* remove old todos
* declare cnabspecversion in parameters pkg

Signed-off-by: Carolyn Van Slyck <me@carolynvanslyck.com>
  • Loading branch information
carolynvs committed Aug 2, 2021
1 parent 60d39d4 commit 0279647
Show file tree
Hide file tree
Showing 7 changed files with 8 additions and 21 deletions.
1 change: 0 additions & 1 deletion pkg/claims/claimstore.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,6 @@ func (s ClaimStore) ListRuns(namespace string, installation string) ([]Run, erro
opts := storage.FindOptions{
Sort: []string{"_id"},
Filter: bson.M{
// TODO(carolynvs): can we consolidate namespace to ns/name on child documents? or installationid? just let claim have a duplicate field, installation + installationid
"namespace": namespace,
"installation": installation,
},
Expand Down
6 changes: 3 additions & 3 deletions pkg/credentials/credentialset.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,20 +7,20 @@ import (
"get.porter.sh/porter/pkg/secrets"
"get.porter.sh/porter/pkg/storage"
"github.com/cnabio/cnab-go/bundle"
cnabcreds "github.com/cnabio/cnab-go/credentials"
"github.com/cnabio/cnab-go/schema"
)

// TODO(carolynvs): use alias
const (
// DefaultSchemaVersion is the default SchemaVersion value
// set on new CredentialSet instances, and is the semver portion
// of CNABSpecVersion.
DefaultSchemaVersion = schema.Version("1.0.0-DRAFT+b6c701f")
DefaultSchemaVersion = cnabcreds.DefaultSchemaVersion

// CNABSpecVersion represents the CNAB Spec version of the Credentials
// that this library implements
// This value is prefixed with e.g. `cnab-credentials-` so isn't itself valid semver.
CNABSpecVersion string = "cnab-credentialsets-" + string(DefaultSchemaVersion)
CNABSpecVersion string = cnabcreds.CNABSpecVersion
)

var _ storage.Document = &CredentialSet{}
Expand Down
2 changes: 0 additions & 2 deletions pkg/encoding/encoding.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,6 @@ func MarshalToml(in interface{}) ([]byte, error) {
// Marshal a struct to the specified format.
// Supported formats are: yaml, json, and toml.
func Marshal(format string, in interface{}) (data []byte, err error) {
// TODO(carolynvs): I wanted to only have to support json marshaler/unmarshaler on our structs but when I converted from json to another format, I was losing the order
// so the resulting file didn't match the order that the struct was defined.
switch format {
case "json":
data, err = json.MarshalIndent(in, "", " ")
Expand Down
2 changes: 1 addition & 1 deletion pkg/porter/credentials.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ type CredentialEditOptions struct {

// ListCredentials lists saved credential sets.
func (p *Porter) ListCredentials(opts ListOptions) error {
// TODO(carolynvs): list by namespace
// TODO(carolynvs): support namespace
creds, err := p.Credentials.ListCredentialSets("")
if err != nil {
return err
Expand Down
1 change: 0 additions & 1 deletion pkg/porter/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,6 @@ func (p *TestPorter) SetupIntegrationTest() {
// update the kubeconfig reference in the credentials to match what's on people's dev machine
ciCredsB = []byte(strings.Replace(string(ciCredsB), "KUBECONFIGPATH", kubeconfig, -1))
var testCreds credentials.CredentialSet
// TODO(carolynvs): do a sweep for yaml.Unmarshal, etc
err = encoding.UnmarshalYaml(ciCredsB, &testCreds)
require.NoError(t, err, "could not unmarshal test credentials %s", ciCredsPath)
err = p.Credentials.UpsertCredentialSet(testCreds)
Expand Down
5 changes: 1 addition & 4 deletions pkg/porter/list.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,6 @@ type DisplayRun struct {
Parameters map[string]interface{} `json:"parameters,omitempty" yaml:"parameters,omitempty"`
Timestamp time.Time `json:"timestamp" yaml:"timestamp"`
Status string `json:"status" yaml:"status"`
HasLogs string `json:"hasLogs" yaml:"hasLogs"`
}

func NewDisplayRun(run claims.Run) DisplayRun {
Expand All @@ -95,10 +94,8 @@ func NewDisplayRun(run claims.Run) DisplayRun {
Timestamp: run.Created,
Bundle: tryParseBundleRepository(run.BundleReference),
Version: run.Bundle.Version,
// TODO(carolynvs): store status on run and record the last result
// TODO(carolynvs): Add command to view all installation runs
//Status: run.GetStatus(),
// TODO(carolynvs): remove for v1, it's a complex lookup now
//HasLogs: hasLogsS,
}
}

Expand Down
12 changes: 3 additions & 9 deletions pkg/storage/migrations/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,7 @@ func (m *Manager) Migrate() (string, error) {
}

if m.ShouldMigrateParameters() {
fmt.Fprintf(w, "Parameters schema is out-of-date (want: %s got: %s)\n", ParameterSetCNABSpecVersion, m.schema.Parameters)
fmt.Fprintf(w, "Parameters schema is out-of-date (want: %s got: %s)\n", parameters.CNABSpecVersion, m.schema.Parameters)
err = m.migrateParameters(w)
migrationErr = multierror.Append(migrationErr, err)
} else {
Expand Down Expand Up @@ -341,7 +341,7 @@ func NewSchema() storage.Schema {
return storage.NewSchema(
schema.Version(claims.CNABSpecVersion),
schema.Version(credentials.CNABSpecVersion),
schema.Version(ParameterSetCNABSpecVersion)) // TODO(carolynvs): standardize like we did for credentials
schema.Version(parameters.CNABSpecVersion))
}

// ShouldMigrateCredentials determines if the credentials storage system requires a migration.
Expand All @@ -353,17 +353,11 @@ func (m *Manager) migrateCredentials(w io.Writer) error {
return nil
}

// TODO(carolynvs): update schmea for documents and set them to porter's own schema
const (
ParameterSetDefaultSchemaVersion schema.Version = "1.0.0-DRAFT+TODO"
ParameterSetCNABSpecVersion string = "cnab-parametersets-" + string(ParameterSetDefaultSchemaVersion)
)

// ShouldMigrateParameters determines if the parameters storage system requires a migration.
func (m *Manager) ShouldMigrateParameters() bool {
// Can't reference parameters.CNABSpecVersion because it causes a circular dependency
// It will be resolved when parametersets move to cnab-go
return string(m.schema.Parameters) != ParameterSetCNABSpecVersion
return string(m.schema.Parameters) != parameters.CNABSpecVersion
}

func (m *Manager) migrateParameters(w io.Writer) error {
Expand Down

0 comments on commit 0279647

Please sign in to comment.