From 37decdfa240a3c210f8cce3978f6dbe2bfe88584 Mon Sep 17 00:00:00 2001 From: Johnny Steenbergen Date: Wed, 30 Oct 2019 10:55:13 -0700 Subject: [PATCH] chore(pkger): fixup loose ends from PR review --- cmd/influx/pkg.go | 50 ++++++++++------- pkger/models.go | 125 +++++++++++++++++++++---------------------- pkger/models_test.go | 46 ++++++++++++++-- pkger/parser.go | 51 +++++++++--------- pkger/service.go | 38 ++++++------- 5 files changed, 177 insertions(+), 133 deletions(-) diff --git a/cmd/influx/pkg.go b/cmd/influx/pkg.go index 05c6f028be..6539fd64b7 100644 --- a/cmd/influx/pkg.go +++ b/cmd/influx/pkg.go @@ -33,13 +33,20 @@ func pkgCmd() *cobra.Command { orgID := cmd.Flags().String("org-id", "", "The ID of the organization that owns the bucket") cmd.MarkFlagRequired("org-id") - cmd.RunE = pkgApply(orgID, path) + hasColor := cmd.Flags().Bool("color", true, "Enable color in output, defaults true") + hasTableBorders := cmd.Flags().Bool("table-borders", true, "Enable table borders, defaults true") + + cmd.RunE = pkgApply(orgID, path, hasColor, hasTableBorders) return cmd } -func pkgApply(orgID, path *string) func(*cobra.Command, []string) error { +func pkgApply(orgID, path *string, hasColor, hasTableBorders *bool) func(*cobra.Command, []string) error { return func(cmd *cobra.Command, args []string) (e error) { + if !*hasColor { + color.NoColor = true + } + influxOrgID, err := influxdb.IDFromString(*orgID) if err != nil { return err @@ -60,7 +67,7 @@ func pkgApply(orgID, path *string) func(*cobra.Command, []string) error { return err } - printPkgDiff(diff) + printPkgDiff(*hasColor, *hasTableBorders, diff) ui := &input.UI{ Writer: os.Stdout, @@ -78,7 +85,7 @@ func pkgApply(orgID, path *string) func(*cobra.Command, []string) error { return err } - printPkgSummary(summary) + printPkgSummary(*hasColor, *hasTableBorders, summary) return nil } @@ -122,7 +129,7 @@ func pkgFromFile(path string) (*pkger.Pkg, error) { return pkger.Parse(enc, pkger.FromFile(path)) } -func printPkgDiff(diff pkger.Diff) { +func printPkgDiff(hasColor, hasTableBorders bool, diff pkger.Diff) { red := color.New(color.FgRed).SprintfFunc() green := color.New(color.FgHiGreen, color.Bold).SprintfFunc() @@ -164,7 +171,7 @@ func printPkgDiff(diff pkger.Diff) { if len(diff.Labels) > 0 { headers := []string{"New", "ID", "Name", "Color", "Description"} - tablePrinter("LABELS", headers, len(diff.Labels), func(w *tablewriter.Table) { + tablePrinter("LABELS", headers, len(diff.Labels), hasColor, hasTableBorders, func(w *tablewriter.Table) { for _, l := range diff.Labels { w.Append([]string{ boolDiff(l.IsNew()), @@ -179,7 +186,7 @@ func printPkgDiff(diff pkger.Diff) { if len(diff.Buckets) > 0 { headers := []string{"New", "ID", "Name", "Retention Period", "Description"} - tablePrinter("BUCKETS", headers, len(diff.Buckets), func(w *tablewriter.Table) { + tablePrinter("BUCKETS", headers, len(diff.Buckets), hasColor, hasTableBorders, func(w *tablewriter.Table) { for _, b := range diff.Buckets { w.Append([]string{ boolDiff(b.IsNew()), @@ -194,7 +201,7 @@ func printPkgDiff(diff pkger.Diff) { if len(diff.LabelMappings) > 0 { headers := []string{"New", "Resource Type", "Resource Name", "Resource ID", "Label Name", "Label ID"} - tablePrinter("LABEL MAPPINGS", headers, len(diff.LabelMappings), func(w *tablewriter.Table) { + tablePrinter("LABEL MAPPINGS", headers, len(diff.LabelMappings), hasColor, hasTableBorders, func(w *tablewriter.Table) { for _, m := range diff.LabelMappings { w.Append([]string{ boolDiff(m.IsNew), @@ -209,10 +216,10 @@ func printPkgDiff(diff pkger.Diff) { } } -func printPkgSummary(sum pkger.Summary) { +func printPkgSummary(hasColor, hasTableBorders bool, sum pkger.Summary) { if labels := sum.Labels; len(labels) > 0 { headers := []string{"ID", "Name", "Description", "Color"} - tablePrinter("LABELS", headers, len(labels), func(w *tablewriter.Table) { + tablePrinter("LABELS", headers, len(labels), hasColor, hasTableBorders, func(w *tablewriter.Table) { for _, l := range labels { w.Append([]string{ l.ID.String(), @@ -226,7 +233,7 @@ func printPkgSummary(sum pkger.Summary) { if buckets := sum.Buckets; len(buckets) > 0 { headers := []string{"ID", "Name", "Retention", "Description"} - tablePrinter("BUCKETS", headers, len(buckets), func(w *tablewriter.Table) { + tablePrinter("BUCKETS", headers, len(buckets), hasColor, hasTableBorders, func(w *tablewriter.Table) { for _, bucket := range buckets { w.Append([]string{ bucket.ID.String(), @@ -240,7 +247,7 @@ func printPkgSummary(sum pkger.Summary) { if mappings := sum.LabelMappings; len(mappings) > 0 { headers := []string{"Resource Type", "Resource Name", "Resource ID", "Label Name", "Label ID"} - tablePrinter("LABEL MAPPINGS", headers, len(mappings), func(w *tablewriter.Table) { + tablePrinter("LABEL MAPPINGS", headers, len(mappings), hasColor, hasTableBorders, func(w *tablewriter.Table) { for _, m := range mappings { w.Append([]string{ string(m.ResourceType), @@ -254,7 +261,7 @@ func printPkgSummary(sum pkger.Summary) { } } -func tablePrinter(table string, headers []string, count int, appendFn func(w *tablewriter.Table)) { +func tablePrinter(table string, headers []string, count int, hasColor, hasTableBorders bool, appendFn func(w *tablewriter.Table)) { descrCol := -1 for i, h := range headers { if strings.ToLower(h) == "description" { @@ -264,7 +271,8 @@ func tablePrinter(table string, headers []string, count int, appendFn func(w *ta } w := tablewriter.NewWriter(os.Stdout) - w.SetBorder(false) + w.SetBorder(hasTableBorders) + w.SetRowLine(hasTableBorders) if descrCol != -1 { w.SetAutoWrapText(false) w.SetColMinWidth(descrCol, 30) @@ -272,11 +280,6 @@ func tablePrinter(table string, headers []string, count int, appendFn func(w *ta color.New(color.FgYellow, color.Bold).Fprintln(os.Stdout, strings.ToUpper(table)) w.SetHeader(headers) - var colors []tablewriter.Colors - for i := 0; i < len(headers); i++ { - colors = append(colors, tablewriter.Color(tablewriter.FgHiCyanColor)) - } - w.SetHeaderColor(colors...) appendFn(w) @@ -284,7 +287,14 @@ func tablePrinter(table string, headers []string, count int, appendFn func(w *ta footers[len(footers)-2] = "TOTAL" footers[len(footers)-1] = strconv.Itoa(count) w.SetFooter(footers) - w.SetFooterColor(colors...) + if hasColor { + var colors []tablewriter.Colors + for i := 0; i < len(headers); i++ { + colors = append(colors, tablewriter.Color(tablewriter.FgHiCyanColor)) + } + w.SetHeaderColor(colors...) + w.SetFooterColor(colors...) + } w.Render() fmt.Fprintln(os.Stdout) diff --git a/pkger/models.go b/pkger/models.go index 5f372c0974..75c9aab86c 100644 --- a/pkger/models.go +++ b/pkger/models.go @@ -106,46 +106,55 @@ type DiffLabelMapping struct { // Summary is a definition of all the resources that have or // will be created from a pkg. type Summary struct { - Buckets []struct { - influxdb.Bucket - Associations []influxdb.Label - } + Buckets []SummaryBucket + Labels []SummaryLabel + LabelMappings []SummaryLabelMapping +} - Labels []struct { - influxdb.Label - } +// SummaryBucket provides a summary of a pkg bucket. +type SummaryBucket struct { + influxdb.Bucket + Associations []influxdb.Label +} - LabelMappings []struct { - ResourceName string - LabelName string - influxdb.LabelMapping - } +// SummaryLabel provides a summary of a pkg label. +type SummaryLabel struct { + influxdb.Label +} + +// SummaryLabelMapping provides a summary of a label mapped with a single resource. +type SummaryLabelMapping struct { + exists bool + ResourceName string + LabelName string + influxdb.LabelMapping } type bucket struct { - ID influxdb.ID + id influxdb.ID OrgID influxdb.ID Description string Name string RetentionPeriod time.Duration labels []*label - // exists provides context for a resource that already - // exists in the platform. If a resource already exists(exists=true) - // then the ID should be populated. + // existing provides context for a resource that already + // exists in the platform. If a resource already exists + // then it will be referenced here. existing *influxdb.Bucket } -func (b *bucket) summarize() struct { - influxdb.Bucket - Associations []influxdb.Label -} { - iBkt := struct { - influxdb.Bucket - Associations []influxdb.Label - }{ +func (b *bucket) ID() influxdb.ID { + if b.existing != nil { + return b.existing.ID + } + return b.id +} + +func (b *bucket) summarize() SummaryBucket { + iBkt := SummaryBucket{ Bucket: influxdb.Bucket{ - ID: b.ID, + ID: b.ID(), OrgID: b.OrgID, Name: b.Name, Description: b.Description, @@ -154,7 +163,7 @@ func (b *bucket) summarize() struct { } for _, l := range b.labels { iBkt.Associations = append(iBkt.Associations, influxdb.Label{ - ID: l.ID, + ID: l.ID(), OrgID: l.OrgID, Name: l.Name, Properties: l.properties(), @@ -182,7 +191,7 @@ func (l labelMapVal) bucket() (*bucket, bool) { } type label struct { - ID influxdb.ID + id influxdb.ID OrgID influxdb.ID Name string Color string @@ -196,32 +205,35 @@ type label struct { existing *influxdb.Label } -func (l *label) mappingSummary() []struct { - exists bool - ResourceName string - LabelName string - influxdb.LabelMapping -} { - var mappings []struct { - exists bool - ResourceName string - LabelName string - influxdb.LabelMapping +func (l *label) ID() influxdb.ID { + if l.existing != nil { + return l.existing.ID } - for k, lm := range l.mappings { - mappings = append(mappings, struct { - exists bool - ResourceName string - LabelName string - influxdb.LabelMapping - }{ + return l.id +} + +func (l *label) summarize() SummaryLabel { + return SummaryLabel{ + Label: influxdb.Label{ + ID: l.ID(), + OrgID: l.OrgID, + Name: l.Name, + Properties: l.properties(), + }, + } +} + +func (l *label) mappingSummary() []SummaryLabelMapping { + var mappings []SummaryLabelMapping + for res, lm := range l.mappings { + mappings = append(mappings, SummaryLabelMapping{ exists: lm.exists, - ResourceName: k.name, + ResourceName: res.name, LabelName: l.Name, LabelMapping: influxdb.LabelMapping{ - LabelID: l.ID, - ResourceID: l.getMappedResourceID(k), - ResourceType: influxdb.BucketsResourceType, + LabelID: l.ID(), + ResourceID: l.getMappedResourceID(res), + ResourceType: res.resType, }, }) } @@ -234,7 +246,7 @@ func (l *label) getMappedResourceID(k labelMapKey) influxdb.ID { case influxdb.BucketsResourceType: b, ok := l.mappings[k].bucket() if ok { - return b.ID + return b.ID() } } return 0 @@ -258,19 +270,6 @@ func (l *label) setBucketMapping(b *bucket, exists bool) { } } -func (l *label) summarize() struct { - influxdb.Label -} { - return struct{ influxdb.Label }{ - Label: influxdb.Label{ - ID: l.ID, - OrgID: l.OrgID, - Name: l.Name, - Properties: l.properties(), - }, - } -} - func (l *label) properties() map[string]string { return map[string]string{ "color": l.Color, diff --git a/pkger/models_test.go b/pkger/models_test.go index 26d745f467..2492fd0906 100644 --- a/pkger/models_test.go +++ b/pkger/models_test.go @@ -16,14 +16,14 @@ func TestPkg(t *testing.T) { pkg := Pkg{ mBuckets: map[string]*bucket{ "buck_2": { - ID: influxdb.ID(2), + id: influxdb.ID(2), OrgID: influxdb.ID(100), Description: "desc2", Name: "name2", RetentionPeriod: 2 * time.Hour, }, "buck_1": { - ID: influxdb.ID(1), + id: influxdb.ID(1), OrgID: influxdb.ID(100), Name: "name1", Description: "desc1", @@ -49,14 +49,14 @@ func TestPkg(t *testing.T) { pkg := Pkg{ mLabels: map[string]*label{ "2": { - ID: influxdb.ID(2), + id: influxdb.ID(2), OrgID: influxdb.ID(100), Name: "name2", Description: "desc2", Color: "blurple", }, "1": { - ID: influxdb.ID(1), + id: influxdb.ID(1), OrgID: influxdb.ID(100), Name: "name1", Description: "desc1", @@ -82,5 +82,43 @@ func TestPkg(t *testing.T) { assert.Equal(t, "name2", label2.Name) assert.Equal(t, "blurple", label2.Properties["color"]) }) + + t.Run("label mappings returned in asc order by name", func(t *testing.T) { + bucket1 := &bucket{ + id: influxdb.ID(20), + Name: "b1", + } + label1 := &label{ + id: influxdb.ID(2), + OrgID: influxdb.ID(100), + Name: "name2", + Description: "desc2", + Color: "blurple", + mappings: map[labelMapKey]labelMapVal{ + labelMapKey{ + resType: influxdb.BucketsResourceType, + name: bucket1.Name, + }: { + v: bucket1, + }, + }, + } + bucket1.labels = append(bucket1.labels, label1) + + pkg := Pkg{ + mBuckets: map[string]*bucket{bucket1.Name: bucket1}, + mLabels: map[string]*label{label1.Name: label1}, + } + + summary := pkg.Summary() + + require.Len(t, summary.LabelMappings, 1) + mapping1 := summary.LabelMappings[0] + assert.Equal(t, bucket1.id, mapping1.ResourceID) + assert.Equal(t, bucket1.Name, mapping1.ResourceName) + assert.Equal(t, influxdb.BucketsResourceType, mapping1.ResourceType) + assert.Equal(t, label1.id, mapping1.LabelID) + assert.Equal(t, label1.Name, mapping1.LabelName) + }) }) } diff --git a/pkger/parser.go b/pkger/parser.go index cc421f2d72..d6762cecca 100644 --- a/pkger/parser.go +++ b/pkger/parser.go @@ -12,7 +12,6 @@ import ( "strings" "time" - "github.com/influxdata/influxdb" "gopkg.in/yaml.v3" ) @@ -30,6 +29,9 @@ const ( EncodingJSON ) +// ErrInvalidEncoding indicates the encoding is invalid type for the parser. +var ErrInvalidEncoding = errors.New("invalid encoding provided") + // Parse parses a pkg defined by the encoding and readerFns. As of writing this // we can parse both a YAML and JSON format of the Pkg model. func Parse(encoding Encoding, readerFn ReaderFn) (*Pkg, error) { @@ -44,8 +46,7 @@ func Parse(encoding Encoding, readerFn ReaderFn) (*Pkg, error) { case EncodingJSON: return parseJSON(r) default: - // TODO: fixup error - return nil, errors.New("invalid encoding provided") + return nil, ErrInvalidEncoding } } @@ -92,13 +93,13 @@ type decoder interface { func parse(dec decoder) (*Pkg, error) { var pkg Pkg - err := dec.Decode(&pkg) - if err != nil { + if err := dec.Decode(&pkg); err != nil { return nil, err } setupFns := []func() error{ pkg.validMetadata, + pkg.validResources, pkg.graphResources, } @@ -152,11 +153,7 @@ func (p *Pkg) Summary() Summary { }) for _, m := range p.labelMappings() { - sum.LabelMappings = append(sum.LabelMappings, struct { - ResourceName string - LabelName string - influxdb.LabelMapping - }{ + sum.LabelMappings = append(sum.LabelMappings, SummaryLabelMapping{ ResourceName: m.ResourceName, LabelName: m.LabelName, LabelMapping: m.LabelMapping, @@ -213,18 +210,8 @@ func (p *Pkg) labels() []*label { // valid pairs of labels and resources of which all have IDs. // If a resource does not exist yet, a label mapping will not // be returned for it. -func (p *Pkg) labelMappings() []struct { - exists bool - ResourceName string - LabelName string - influxdb.LabelMapping -} { - var mappings []struct { - exists bool - ResourceName string - LabelName string - influxdb.LabelMapping - } +func (p *Pkg) labelMappings() []SummaryLabelMapping { + var mappings []SummaryLabelMapping for _, l := range p.mLabels { mappings = append(mappings, l.mappingSummary()...) } @@ -285,6 +272,24 @@ func (p *Pkg) validMetadata() error { return &err } +func (p *Pkg) validResources() error { + if len(p.Spec.Resources) > 0 { + return nil + } + + res := errResource{ + Type: "Package", + Idx: -1, + } + res.ValidationFails = append(res.ValidationFails, struct { + Field string + Msg string + }{Field: "resources", Msg: "at least 1 resource must be provided"}) + var err ParseErr + err.append(res) + return &err +} + func (p *Pkg) graphResources() error { graphFns := []func() error{ // labels are first to validate associations with other resources @@ -298,8 +303,6 @@ func (p *Pkg) graphResources() error { } } - // TODO: make sure a resource was created.... - return nil } diff --git a/pkger/service.go b/pkger/service.go index f0ad7f5041..7940417756 100644 --- a/pkger/service.go +++ b/pkger/service.go @@ -53,9 +53,9 @@ func (s *Service) DryRun(ctx context.Context, orgID influxdb.ID, pkg *Pkg) (Summ return Summary{}, Diff{}, err } - // verify the pkg is verified by a dry run. when calling apply this + // verify the pkg is verified by a dry run. when calling Service.Apply this // is required to have been run. if it is not true, then apply runs - // the Dry run again. + // the Dry run. pkg.isVerified = true diff := Diff{ @@ -73,13 +73,10 @@ func (s *Service) dryRunBuckets(ctx context.Context, orgID influxdb.ID, pkg *Pkg b := bkts[i] existingBkt, err := s.bucketSVC.FindBucketByName(ctx, orgID, b.Name) switch err { - // TODO: this is useless until the http client provides this functionality, right none of the http - // clients do :sad_face: // TODO: case for err not found here and another case handle where // err isn't a not found (some other error) case nil: b.existing = existingBkt - b.ID = existingBkt.ID mExistingBkts[b.Name] = newDiffBucket(b, *existingBkt) default: mExistingBkts[b.Name] = newDiffBucket(b, influxdb.Bucket{}) @@ -107,14 +104,11 @@ func (s *Service) dryRunLabels(ctx context.Context, orgID influxdb.ID, pkg *Pkg) OrgID: &orgID, }, influxdb.FindOptions{Limit: 1}) switch { - // TODO: this is useless until the http client provides this functionality, right none of the http - // clients do :sad_face: // TODO: case for err not found here and another case handle where // err isn't a not found (some other error) case err == nil && len(existingLabels) > 0: existingLabel := existingLabels[0] l.existing = existingLabel - l.ID = existingLabel.ID mExistingLabels[l.Name] = newDiffLabel(l, *existingLabel) default: mExistingLabels[l.Name] = newDiffLabel(l, influxdb.Label{}) @@ -140,7 +134,7 @@ func (s *Service) dryRunLabelMappings(ctx context.Context, pkg *Pkg) ([]DiffLabe diffs = append(diffs, DiffLabelMapping{ IsNew: isNew, ResType: influxdb.BucketsResourceType, - ResID: b.ID, + ResID: b.ID(), ResName: b.Name, LabelID: labelID, LabelName: labelName, @@ -177,7 +171,7 @@ type labelMappingDiffFn func(labelID influxdb.ID, labelName string, isNew bool) func (s *Service) dryRunBucketLabelMapping(ctx context.Context, b *bucket, mappingFn labelMappingDiffFn) error { if b.existing == nil { for _, l := range b.labels { - mappingFn(l.ID, l.Name, true) + mappingFn(l.ID(), l.Name, true) } return nil } @@ -185,7 +179,7 @@ func (s *Service) dryRunBucketLabelMapping(ctx context.Context, b *bucket, mappi // lookup labels in pkg, add it to the label mapping, if exists in // the results from API, mark it exists labels, err := s.labelSVC.FindResourceLabels(ctx, influxdb.LabelMappingFilter{ - ResourceID: b.ID, + ResourceID: b.ID(), ResourceType: influxdb.BucketsResourceType, }) if err != nil { @@ -203,7 +197,7 @@ func (s *Service) dryRunBucketLabelMapping(ctx context.Context, b *bucket, mappi // now we add labels that were not apart of the existing labels for _, l := range pkgLabels { - mappingFn(l.ID, l.Name, true) + mappingFn(l.ID(), l.Name, true) } return nil } @@ -277,7 +271,7 @@ func (s *Service) applyBuckets(buckets []*bucket) applier { }) continue } - buckets[i].ID = influxBucket.ID + buckets[i].id = influxBucket.ID rollbackBuckets = append(rollbackBuckets, buckets[i]) } @@ -297,19 +291,19 @@ func (s *Service) rollbackBuckets(buckets []*bucket) error { var errs []string for _, b := range buckets { if b.existing == nil { - err := s.bucketSVC.DeleteBucket(context.Background(), b.ID) + err := s.bucketSVC.DeleteBucket(context.Background(), b.ID()) if err != nil { - errs = append(errs, b.ID.String()) + errs = append(errs, b.ID().String()) } continue } - _, err := s.bucketSVC.UpdateBucket(context.Background(), b.ID, influxdb.BucketUpdate{ + _, err := s.bucketSVC.UpdateBucket(context.Background(), b.ID(), influxdb.BucketUpdate{ Description: &b.Description, RetentionPeriod: &b.RetentionPeriod, }) if err != nil { - errs = append(errs, b.ID.String()) + errs = append(errs, b.ID().String()) } } @@ -323,7 +317,7 @@ func (s *Service) rollbackBuckets(buckets []*bucket) error { func (s *Service) applyBucket(ctx context.Context, b *bucket) (influxdb.Bucket, error) { if b.existing != nil { - influxBucket, err := s.bucketSVC.UpdateBucket(ctx, b.ID, influxdb.BucketUpdate{ + influxBucket, err := s.bucketSVC.UpdateBucket(ctx, b.ID(), influxdb.BucketUpdate{ Description: &b.Description, RetentionPeriod: &b.RetentionPeriod, }) @@ -364,7 +358,7 @@ func (s *Service) applyLabels(labels []*label) applier { }) continue } - labels[i].ID = influxLabel.ID + labels[i].id = influxLabel.ID rollBackLabels = append(rollBackLabels, labels[i]) } @@ -383,9 +377,9 @@ func (s *Service) applyLabels(labels []*label) applier { func (s *Service) rollbackLabels(labels []*label) error { var errs []string for _, l := range labels { - err := s.labelSVC.DeleteLabel(context.Background(), l.ID) + err := s.labelSVC.DeleteLabel(context.Background(), l.ID()) if err != nil { - errs = append(errs, l.ID.String()) + errs = append(errs, l.ID().String()) } } @@ -398,7 +392,7 @@ func (s *Service) rollbackLabels(labels []*label) error { func (s *Service) applyLabel(ctx context.Context, l *label) (influxdb.Label, error) { if l.existing != nil { - updatedlabel, err := s.labelSVC.UpdateLabel(ctx, l.ID, influxdb.LabelUpdate{ + updatedlabel, err := s.labelSVC.UpdateLabel(ctx, l.ID(), influxdb.LabelUpdate{ Properties: l.properties(), }) if err != nil {