From 43f02c59e8a23b3cc258d16636451e649db63c1a Mon Sep 17 00:00:00 2001 From: Dustin Scott Date: Tue, 14 Jun 2022 05:10:55 -0500 Subject: [PATCH 1/3] fix: Fixes #32, added support for integer replacement Signed-off-by: Dustin Scott --- internal/workload/v1/markers/markers.go | 18 +++++-- .../v1/markers/markers_internal_test.go | 50 ++++++++++++++++--- 2 files changed, 59 insertions(+), 9 deletions(-) diff --git a/internal/workload/v1/markers/markers.go b/internal/workload/v1/markers/markers.go index 31e4b05..8f7633f 100644 --- a/internal/workload/v1/markers/markers.go +++ b/internal/workload/v1/markers/markers.go @@ -175,8 +175,15 @@ func isReserved(fieldName string) bool { // getSourceCodeFieldVariable gets a full variable name for a marker as it is intended to be // passed into the generate package to generate the source code. This includes particular // tags that are needed by the generator to properly identify when a variable starts and ends. -func getSourceCodeFieldVariable(marker FieldMarkerProcessor) string { - return fmt.Sprintf("!!start %s !!end", marker.GetSourceCodeVariable()) +func getSourceCodeFieldVariable(marker FieldMarkerProcessor) (string, error) { + switch marker.GetFieldType() { + case FieldString: + return fmt.Sprintf("!!start %s !!end", marker.GetSourceCodeVariable()), nil + case FieldInt: + return fmt.Sprintf("!!start string(rune(%s)) !!end", marker.GetSourceCodeVariable()), nil + default: + return "", fmt.Errorf("cannot handle field type: %s", marker.GetFieldType()) + } } // getSourceCodeVariable gets a full variable name for a marker as it is intended to be @@ -240,7 +247,12 @@ func setValue(marker FieldMarkerProcessor, value *yaml.Node) error { return fmt.Errorf("unable to convert %s to regex, %w", markerReplaceText, err) } - value.Value = re.ReplaceAllString(value.Value, getSourceCodeFieldVariable(marker)) + fieldVar, err := getSourceCodeFieldVariable(marker) + if err != nil { + return fmt.Errorf("unable to get source code field variable for marker %s, %w", marker, err) + } + + value.Value = re.ReplaceAllString(value.Value, fieldVar) } else { value.Tag = varTag value.Value = marker.GetSourceCodeVariable() diff --git a/internal/workload/v1/markers/markers_internal_test.go b/internal/workload/v1/markers/markers_internal_test.go index 4755af2..b77f279 100644 --- a/internal/workload/v1/markers/markers_internal_test.go +++ b/internal/workload/v1/markers/markers_internal_test.go @@ -219,11 +219,25 @@ func Test_getSourceCodeFieldVariable(t *testing.T) { fieldMarkerTest := &FieldMarker{ Name: "field.marker", sourceCodeVar: "parent.Spec.Field.Marker", + Type: FieldString, } collectionFieldMarkerTest := &CollectionFieldMarker{ Name: "collection", sourceCodeVar: "collection.Spec.Collection", + Type: FieldString, + } + + intTest := &FieldMarker{ + Name: "field.integer", + sourceCodeVar: "parent.Spec.Field.Integer", + Type: FieldInt, + } + + failTest := &FieldMarker{ + Name: "field.fail", + sourceCodeVar: "parent.Spec.Field.Fail", + Type: FieldBool, } type args struct { @@ -231,23 +245,42 @@ func Test_getSourceCodeFieldVariable(t *testing.T) { } tests := []struct { - name string - args args - want string + name string + args args + want string + wantErr bool }{ { name: "ensure field marker returns a correct source code variable field", args: args{ marker: fieldMarkerTest, }, - want: "!!start parent.Spec.Field.Marker !!end", + want: "!!start parent.Spec.Field.Marker !!end", + wantErr: false, }, { name: "ensure collection field marker returns a correct source code variable field", args: args{ marker: collectionFieldMarkerTest, }, - want: "!!start collection.Spec.Collection !!end", + want: "!!start collection.Spec.Collection !!end", + wantErr: false, + }, + { + name: "ensure integer field marker returns a correct source code variable field", + args: args{ + marker: intTest, + }, + want: "!!start rune(parent.Spec.Field.Integer) !!end", + wantErr: false, + }, + { + name: "ensure unsupported field marker returns an error", + args: args{ + marker: failTest, + }, + want: "", + wantErr: true, }, } @@ -255,7 +288,12 @@ func Test_getSourceCodeFieldVariable(t *testing.T) { tt := tt t.Run(tt.name, func(t *testing.T) { t.Parallel() - if got := getSourceCodeFieldVariable(tt.args.marker); got != tt.want { + got, err := getSourceCodeFieldVariable(tt.args.marker) + if (err != nil) != tt.wantErr { + t.Errorf("getSourceCodeFieldVariable() error = %v, wantErr %v", err, tt.wantErr) + } + + if got != tt.want { t.Errorf("getSourceCodeFieldVariable() = %v, want %v", got, tt.want) } }) From dc098a7a51a98930f24ff5a248d16797cb96e6ef Mon Sep 17 00:00:00 2001 From: Dustin Scott Date: Tue, 14 Jun 2022 07:06:24 -0500 Subject: [PATCH 2/3] fix: Fixes #18, added ability to use parent resource name in child resource Signed-off-by: Dustin Scott --- docs/markers.md | 9 +- internal/workload/v1/kinds/workload.go | 18 +-- .../v1/markers/collection_field_marker.go | 21 ++- .../collection_field_marker_internal_test.go | 20 +-- internal/workload/v1/markers/field_marker.go | 24 +++- .../v1/markers/field_marker_internal_test.go | 20 +-- internal/workload/v1/markers/markers.go | 61 +++++++-- .../v1/markers/markers_internal_test.go | 127 ++++++++++++++---- .../workload/v1/markers/resource_marker.go | 22 ++- .../markers/resource_marker_internal_test.go | 48 +++---- 10 files changed, 282 insertions(+), 88 deletions(-) diff --git a/docs/markers.md b/docs/markers.md index 54a823e..6d33b5a 100644 --- a/docs/markers.md +++ b/docs/markers.md @@ -36,7 +36,7 @@ field for your workload. | [replace](#replace-optional) | string | false | | [description](#description-optional) | string | false | -### Name (required) +### Name (required if Parent is unspecified) The name you want to use for the field in the custom resource that Operator Builder will create. If you're not sure what that means, it will @@ -44,6 +44,13 @@ become clear shortly. ex. +operator-builder:field:name=myName +### Parent (required if Name is unspecified) + +The parent field in which you wish to substitute. Currently, only `metadata.name` is supported. This +will allow you to use the parent name as a value in the child resource. + +ex. +operator-builder:field:parent=metadata.name + ### Type (required) The other required field is the `type` field which specifies the data type for diff --git a/internal/workload/v1/kinds/workload.go b/internal/workload/v1/kinds/workload.go index d7a0741..9e488d7 100644 --- a/internal/workload/v1/kinds/workload.go +++ b/internal/workload/v1/kinds/workload.go @@ -364,14 +364,16 @@ func (ws *WorkloadSpec) processMarkerResults(markerResults []*inspect.YAMLResult } // add the field to the api specification - if err := ws.APISpecFields.AddField( - marker.GetName(), - marker.GetFieldType(), - comments, - sampleVal, - defaultFound, - ); err != nil { - return err + if marker.GetName() != "" { + if err := ws.APISpecFields.AddField( + marker.GetName(), + marker.GetFieldType(), + comments, + sampleVal, + defaultFound, + ); err != nil { + return err + } } marker.SetForCollection(ws.ForCollection) diff --git a/internal/workload/v1/markers/collection_field_marker.go b/internal/workload/v1/markers/collection_field_marker.go index 21e1ef1..7601a0a 100644 --- a/internal/workload/v1/markers/collection_field_marker.go +++ b/internal/workload/v1/markers/collection_field_marker.go @@ -11,6 +11,7 @@ import ( const ( CollectionFieldMarkerPrefix = "+operator-builder:collection:field" + CollectionFieldPrefix = "collection" CollectionFieldSpecPrefix = "collection.Spec" ) @@ -24,7 +25,7 @@ type CollectionFieldMarker FieldMarker //nolint:gocritic //needed to implement string interface func (cfm CollectionFieldMarker) String() string { return fmt.Sprintf("CollectionFieldMarker{Name: %s Type: %v Description: %q Default: %v}", - cfm.Name, + cfm.GetName(), cfm.Type, cfm.GetDescription(), cfm.Default, @@ -47,7 +48,11 @@ func defineCollectionFieldMarker(registry *marker.Registry) error { // FieldMarkerProcessor interface methods. // func (cfm *CollectionFieldMarker) GetName() string { - return cfm.Name + if cfm.Name == nil { + return "" + } + + return *cfm.Name } func (cfm *CollectionFieldMarker) GetDefault() interface{} { @@ -74,6 +79,10 @@ func (cfm *CollectionFieldMarker) GetReplaceText() string { return *cfm.Replace } +func (cfm *CollectionFieldMarker) GetPrefix() string { + return CollectionFieldPrefix +} + func (cfm *CollectionFieldMarker) GetSpecPrefix() string { return CollectionFieldSpecPrefix } @@ -86,6 +95,14 @@ func (cfm *CollectionFieldMarker) GetOriginalValue() interface{} { return cfm.originalValue } +func (cfm *CollectionFieldMarker) GetParent() string { + if cfm.Parent == nil { + return "" + } + + return *cfm.Parent +} + func (cfm *CollectionFieldMarker) IsCollectionFieldMarker() bool { return true } diff --git a/internal/workload/v1/markers/collection_field_marker_internal_test.go b/internal/workload/v1/markers/collection_field_marker_internal_test.go index 376b241..3cb3bdc 100644 --- a/internal/workload/v1/markers/collection_field_marker_internal_test.go +++ b/internal/workload/v1/markers/collection_field_marker_internal_test.go @@ -15,9 +15,10 @@ func TestCollectionFieldMarker_String(t *testing.T) { t.Parallel() testString := "cfm test" + testName := "test" type fields struct { - Name string + Name *string Type FieldType Description *string Default interface{} @@ -31,7 +32,7 @@ func TestCollectionFieldMarker_String(t *testing.T) { { name: "ensure collection field string output matches expected", fields: fields{ - Name: "test", + Name: &testName, Type: FieldString, Description: &testString, Default: "test", @@ -41,7 +42,7 @@ func TestCollectionFieldMarker_String(t *testing.T) { { name: "ensure collection field with nil values output matches expected", fields: fields{ - Name: "test", + Name: &testName, Type: FieldString, Description: nil, Default: "test", @@ -143,11 +144,14 @@ func TestCollectionFieldMarker_GetDefault(t *testing.T) { } } -func TestCollectionFieldMarker_GetDescription(t *testing.T) { +func TestCollectionFieldMarker_GetName(t *testing.T) { t.Parallel() + name := "test" + emptyName := "" + type fields struct { - Name string + Name *string } tests := []struct { @@ -158,14 +162,14 @@ func TestCollectionFieldMarker_GetDescription(t *testing.T) { { name: "ensure collection field name returns as expected", fields: fields{ - Name: "test", + Name: &name, }, want: "test", }, { name: "ensure collection field name with empty value returns as expected", fields: fields{ - Name: "", + Name: &emptyName, }, want: "", }, @@ -185,7 +189,7 @@ func TestCollectionFieldMarker_GetDescription(t *testing.T) { } } -func TestCollectionFieldMarker_GetName(t *testing.T) { +func TestCollectionFieldMarker_GetDescription(t *testing.T) { t.Parallel() cfmDescription := "test collection description" diff --git a/internal/workload/v1/markers/field_marker.go b/internal/workload/v1/markers/field_marker.go index 5ad7f4a..4f7889b 100644 --- a/internal/workload/v1/markers/field_marker.go +++ b/internal/workload/v1/markers/field_marker.go @@ -17,6 +17,7 @@ var ( const ( FieldMarkerPrefix = "+operator-builder:field" + FieldPrefix = "parent" FieldSpecPrefix = "parent.Spec" ) @@ -25,11 +26,12 @@ const ( // and matches the constants defined by the fieldMarker constant above. type FieldMarker struct { // inputs from the marker itself - Name string + Name *string Type FieldType Description *string Default interface{} `marker:",optional"` Replace *string + Parent *string // other values which we use to pass information forCollection bool @@ -40,7 +42,7 @@ type FieldMarker struct { //nolint:gocritic //needed to implement string interface func (fm FieldMarker) String() string { return fmt.Sprintf("FieldMarker{Name: %s Type: %v Description: %q Default: %v}", - fm.Name, + fm.GetName(), fm.Type, fm.GetDescription(), fm.Default, @@ -63,7 +65,11 @@ func defineFieldMarker(registry *marker.Registry) error { // FieldMarker Processor interface methods. // func (fm *FieldMarker) GetName() string { - return fm.Name + if fm.Name == nil { + return "" + } + + return *fm.Name } func (fm *FieldMarker) GetDefault() interface{} { @@ -90,6 +96,10 @@ func (fm *FieldMarker) GetReplaceText() string { return *fm.Replace } +func (fm *FieldMarker) GetPrefix() string { + return FieldPrefix +} + func (fm *FieldMarker) GetSpecPrefix() string { return FieldSpecPrefix } @@ -98,6 +108,14 @@ func (fm *FieldMarker) GetOriginalValue() interface{} { return fm.originalValue } +func (fm *FieldMarker) GetParent() string { + if fm.Parent == nil { + return "" + } + + return *fm.Parent +} + func (fm *FieldMarker) GetSourceCodeVariable() string { return fm.sourceCodeVar } diff --git a/internal/workload/v1/markers/field_marker_internal_test.go b/internal/workload/v1/markers/field_marker_internal_test.go index 1f11cbb..471791c 100644 --- a/internal/workload/v1/markers/field_marker_internal_test.go +++ b/internal/workload/v1/markers/field_marker_internal_test.go @@ -14,10 +14,11 @@ import ( func TestFieldMarker_String(t *testing.T) { t.Parallel() + testName := "test" testString := "fm test" type fields struct { - Name string + Name *string Type FieldType Description *string Default interface{} @@ -31,7 +32,7 @@ func TestFieldMarker_String(t *testing.T) { { name: "ensure field string output matches expected", fields: fields{ - Name: "test", + Name: &testName, Type: FieldString, Description: &testString, Default: "test", @@ -41,7 +42,7 @@ func TestFieldMarker_String(t *testing.T) { { name: "ensure field with nil values output matches expected", fields: fields{ - Name: "test", + Name: &testName, Type: FieldString, Description: nil, Default: "test", @@ -143,11 +144,14 @@ func TestFieldMarker_GetDefault(t *testing.T) { } } -func TestFieldMarker_GetDescription(t *testing.T) { +func TestFieldMarker_GetName(t *testing.T) { t.Parallel() + name := "test" + emptyName := "" + type fields struct { - Name string + Name *string } tests := []struct { @@ -158,14 +162,14 @@ func TestFieldMarker_GetDescription(t *testing.T) { { name: "ensure field name returns as expected", fields: fields{ - Name: "test", + Name: &name, }, want: "test", }, { name: "ensure field name with empty value returns as expected", fields: fields{ - Name: "", + Name: &emptyName, }, want: "", }, @@ -185,7 +189,7 @@ func TestFieldMarker_GetDescription(t *testing.T) { } } -func TestFieldMarker_GetName(t *testing.T) { +func TestFieldMarker_GetDescription(t *testing.T) { t.Parallel() fmDescription := "test description" diff --git a/internal/workload/v1/markers/markers.go b/internal/workload/v1/markers/markers.go index 8f7633f..6f7b05c 100644 --- a/internal/workload/v1/markers/markers.go +++ b/internal/workload/v1/markers/markers.go @@ -32,6 +32,7 @@ type FieldMarkerProcessor interface { GetDescription() string GetFieldType() FieldType GetOriginalValue() interface{} + GetParent() string GetReplaceText() string GetSpecPrefix() string GetSourceCodeVariable() string @@ -49,7 +50,9 @@ type FieldMarkerProcessor interface { // necessary for parsing any type of marker. type MarkerProcessor interface { GetName() string + GetPrefix() string GetSpecPrefix() string + GetParent() string } // MarkerCollection is an object that stores a set of markers. @@ -121,15 +124,30 @@ func transformYAML(results ...*inspect.YAMLResult) error { switch t := result.Object.(type) { case FieldMarker: - t.sourceCodeVar = getSourceCodeVariable(&t) + sourceCodeVar, err := getSourceCodeVariable(&t) + if err != nil { + return err + } + + t.sourceCodeVar = sourceCodeVar marker = &t case CollectionFieldMarker: - t.sourceCodeVar = getSourceCodeVariable(&t) + sourceCodeVar, err := getSourceCodeVariable(&t) + if err != nil { + return err + } + + t.sourceCodeVar = sourceCodeVar marker = &t default: continue } + // ensure that either a parent or a name is set + if marker.GetName() == "" && marker.GetParent() == "" { + return fmt.Errorf("must set either parent=value or name=value for marker %s", marker) + } + // get common variables and confirm that we are not working with a reserved marker if isReserved(marker.GetName()) { return fmt.Errorf("%s %w", marker.GetName(), ErrFieldMarkerReserved) @@ -160,11 +178,30 @@ func reservedMarkers() []string { } } +// supportedParents represents a map of parent fields to their go variable values +// which are currently supported. +func supportedParents() map[string]string { + return map[string]string{ + "metadata.name": "Name", + } +} + // isReserved is a convenience method which returns whether or not a marker, given // the fieldName as a string, is reserved for internal purposes. func isReserved(fieldName string) bool { - for _, reserved := range reservedMarkers() { - if strings.Title(fieldName) == strings.Title(reserved) { + return validField(fieldName, reservedMarkers()) +} + +// isSupported is a convenience method which returns whether or not a marker, given +// the parentField as a string, is supported. +func isSupported(parentField string) bool { + return supportedParents()[parentField] != "" +} + +// validField determines if a field is valid based on a known list of valid fields. +func validField(field string, validFields []string) bool { + for _, valid := range validFields { + if strings.Title(valid) == strings.Title(field) { return true } } @@ -188,8 +225,16 @@ func getSourceCodeFieldVariable(marker FieldMarkerProcessor) (string, error) { // getSourceCodeVariable gets a full variable name for a marker as it is intended to be // scaffolded in the source code. -func getSourceCodeVariable(marker MarkerProcessor) string { - return fmt.Sprintf("%s.%s", marker.GetSpecPrefix(), strings.Title((marker.GetName()))) +func getSourceCodeVariable(marker MarkerProcessor) (string, error) { + if marker.GetParent() == "" { + return fmt.Sprintf("%s.%s", marker.GetSpecPrefix(), strings.Title((marker.GetName()))), nil + } + + if isSupported(marker.GetParent()) { + return fmt.Sprintf("%s.%s", marker.GetPrefix(), supportedParents()[marker.GetParent()]), nil + } + + return "", fmt.Errorf("parent field requested %s but is not supported. supported parent fields are: %v", marker.GetParent(), supportedParents()) } // getKeyValue gets the key and value from a YAML result. @@ -217,9 +262,9 @@ func setComments(marker FieldMarkerProcessor, result *inspect.YAMLResult, key, v var appendText string switch t := marker.(type) { case *FieldMarker: - appendText = "controlled by field: " + t.Name + appendText = "controlled by field: " + t.GetName() case *CollectionFieldMarker: - appendText = "controlled by collection field: " + t.Name + appendText = "controlled by collection field: " + t.GetName() } // set the comments on the yaml nodes diff --git a/internal/workload/v1/markers/markers_internal_test.go b/internal/workload/v1/markers/markers_internal_test.go index b77f279..7c49325 100644 --- a/internal/workload/v1/markers/markers_internal_test.go +++ b/internal/workload/v1/markers/markers_internal_test.go @@ -216,26 +216,31 @@ func Test_isReserved(t *testing.T) { func Test_getSourceCodeFieldVariable(t *testing.T) { t.Parallel() + fieldMarkerTestString := "field.marker" + collectionFieldMarkerTestString := "collection" + intMarkerTest := "field.integer" + failTestString := "field.fail" + fieldMarkerTest := &FieldMarker{ - Name: "field.marker", + Name: &fieldMarkerTestString, sourceCodeVar: "parent.Spec.Field.Marker", Type: FieldString, } collectionFieldMarkerTest := &CollectionFieldMarker{ - Name: "collection", + Name: &collectionFieldMarkerTestString, sourceCodeVar: "collection.Spec.Collection", Type: FieldString, } intTest := &FieldMarker{ - Name: "field.integer", + Name: &intMarkerTest, sourceCodeVar: "parent.Spec.Field.Integer", Type: FieldInt, } failTest := &FieldMarker{ - Name: "field.fail", + Name: &failTestString, sourceCodeVar: "parent.Spec.Field.Fail", Type: FieldBool, } @@ -271,7 +276,7 @@ func Test_getSourceCodeFieldVariable(t *testing.T) { args: args{ marker: intTest, }, - want: "!!start rune(parent.Spec.Field.Integer) !!end", + want: "!!start string(rune(parent.Spec.Field.Integer)) !!end", wantErr: false, }, { @@ -303,12 +308,34 @@ func Test_getSourceCodeFieldVariable(t *testing.T) { func Test_getSourceCodeVariable(t *testing.T) { t.Parallel() + highlyNested := "this.is.a.highly.nested.field" + flat := "flat" + fieldMarkerTest := &FieldMarker{ - Name: "this.is.a.highly.nested.field", + Name: &highlyNested, } collectionFieldMarkerTest := &CollectionFieldMarker{ - Name: "flat", + Name: &flat, + } + + validParent := "metadata.name" + invalidParent := "metadata.namespace" + + fieldMarkerParentTest := &FieldMarker{ + Parent: &validParent, + } + + fieldMarkerInvalidParentTest := &FieldMarker{ + Parent: &invalidParent, + } + + collectionFieldMarkerParentTest := &CollectionFieldMarker{ + Parent: &validParent, + } + + collectionFieldMarkerInvalidParentTest := &CollectionFieldMarker{ + Parent: &invalidParent, } fieldMarkerField := "test.field.marker.field" @@ -327,37 +354,74 @@ func Test_getSourceCodeVariable(t *testing.T) { } tests := []struct { - name string - args args - want string + name string + args args + want string + wantErr bool }{ { name: "ensure field marker returns a correct source code variable", args: args{ marker: fieldMarkerTest, }, - want: "parent.Spec.This.Is.A.Highly.Nested.Field", + want: "parent.Spec.This.Is.A.Highly.Nested.Field", + wantErr: false, }, { name: "ensure collection field marker returns a correct source code variable", args: args{ marker: collectionFieldMarkerTest, }, - want: "collection.Spec.Flat", + want: "collection.Spec.Flat", + wantErr: false, }, { name: "ensure resource marker with field marker field returns a correct source code variable", args: args{ marker: resourceMarkerFieldTest, }, - want: "parent.Spec.Test.Field.Marker.Field", + want: "parent.Spec.Test.Field.Marker.Field", + wantErr: false, }, { name: "ensure resource marker with collection field marker field returns a correct source code variable", args: args{ marker: resourceMarkerCollectionFieldTest, }, - want: "collection.Spec.Test.Collection.Field.Marker.Field", + want: "collection.Spec.Test.Collection.Field.Marker.Field", + wantErr: false, + }, + { + name: "ensure field marker with parent returns a correct source code variable", + args: args{ + marker: fieldMarkerParentTest, + }, + want: "parent.Name", + wantErr: false, + }, + { + name: "ensure field marker with invalid parent returns an error", + args: args{ + marker: fieldMarkerInvalidParentTest, + }, + want: "", + wantErr: true, + }, + { + name: "ensure collection field marker with parent returns a correct source code variable", + args: args{ + marker: collectionFieldMarkerParentTest, + }, + want: "collection.Name", + wantErr: false, + }, + { + name: "ensure collection field marker with invalid parent returns an error", + args: args{ + marker: collectionFieldMarkerInvalidParentTest, + }, + want: "", + wantErr: true, }, } @@ -365,7 +429,12 @@ func Test_getSourceCodeVariable(t *testing.T) { tt := tt t.Run(tt.name, func(t *testing.T) { t.Parallel() - if got := getSourceCodeVariable(tt.args.marker); got != tt.want { + got, err := getSourceCodeVariable(tt.args.marker) + if (err != nil) != tt.wantErr { + t.Errorf("getSourceCodeVariable() error = %v, wantErr %v", err, tt.wantErr) + } + + if got != tt.want { t.Errorf("getSourceCodeVariable() = %v, want %v", got, tt.want) } }) @@ -441,6 +510,7 @@ func Test_setValue(t *testing.T) { //nolint: goconst testInvalidReplaceText := "*&^%" testReplaceText := "" + testField := "test.field" type args struct { marker FieldMarkerProcessor @@ -457,8 +527,9 @@ func Test_setValue(t *testing.T) { name: "ensure value is set appropriately when replace text is not requested", args: args{ marker: &FieldMarker{ - Name: "test.field", + Name: &testField, sourceCodeVar: "parent.Spec.Test.Field", + Type: FieldString, }, value: &yaml.Node{ Tag: "testTag", @@ -475,9 +546,10 @@ func Test_setValue(t *testing.T) { name: "ensure value is set appropriately when replace text is requested", args: args{ marker: &FieldMarker{ - Name: "test.field", + Name: &testField, Replace: &testReplaceText, sourceCodeVar: "parent.Spec.Test.Field", + Type: FieldString, }, value: &yaml.Node{ Tag: "testTag", @@ -494,9 +566,10 @@ func Test_setValue(t *testing.T) { name: "ensure invalid replace text returns an error", args: args{ marker: &FieldMarker{ - Name: "test.field", + Name: &testField, Replace: &testInvalidReplaceText, sourceCodeVar: "parent.Spec.Test.Field", + Type: FieldString, }, value: &yaml.Node{ Tag: "testTag", @@ -548,7 +621,7 @@ func Test_setComments(t *testing.T) { name: "ensure head comment is set correctly with a description", args: args{ marker: &FieldMarker{ - Name: testName, + Name: &testName, Description: &testHeadCommentDescription, }, result: &inspect.YAMLResult{ @@ -572,7 +645,7 @@ func Test_setComments(t *testing.T) { name: "ensure head comment is set correctly without a description", args: args{ marker: &FieldMarker{ - Name: testName, + Name: &testName, }, result: &inspect.YAMLResult{ Result: &parser.Result{ @@ -595,7 +668,7 @@ func Test_setComments(t *testing.T) { name: "ensure line comment is set correctly with a description", args: args{ marker: &CollectionFieldMarker{ - Name: testName, + Name: &testName, Description: &testHeadCommentDescription, }, result: &inspect.YAMLResult{ @@ -618,7 +691,7 @@ func Test_setComments(t *testing.T) { name: "ensure line comment is set correctly without a description", args: args{ marker: &CollectionFieldMarker{ - Name: testName, + Name: &testName, }, result: &inspect.YAMLResult{ Result: &parser.Result{ @@ -657,6 +730,8 @@ func Test_transformYAML(t *testing.T) { t.Parallel() badReplaceText := "*&^%" + realField := "real.field" + collectionName := "collection.name" type args struct { results []*inspect.YAMLResult @@ -675,7 +750,7 @@ func Test_transformYAML(t *testing.T) { Result: &parser.Result{ MarkerText: "test", Object: FieldMarker{ - Name: "real.field", + Name: &realField, }, }, Nodes: []*yaml.Node{ @@ -711,7 +786,7 @@ func Test_transformYAML(t *testing.T) { Result: &parser.Result{ MarkerText: "test", Object: FieldMarker{ - Name: "collection.name", + Name: &collectionName, }, }, }, @@ -727,7 +802,7 @@ func Test_transformYAML(t *testing.T) { Result: &parser.Result{ MarkerText: "test", Object: CollectionFieldMarker{ - Name: "collection.name", + Name: &collectionName, }, }, }, @@ -743,7 +818,7 @@ func Test_transformYAML(t *testing.T) { Result: &parser.Result{ MarkerText: "test", Object: CollectionFieldMarker{ - Name: "real.field", + Name: &realField, Replace: &badReplaceText, }, }, diff --git a/internal/workload/v1/markers/resource_marker.go b/internal/workload/v1/markers/resource_marker.go index 1b10062..e561561 100644 --- a/internal/workload/v1/markers/resource_marker.go +++ b/internal/workload/v1/markers/resource_marker.go @@ -127,6 +127,16 @@ func (rm *ResourceMarker) GetField() string { return *rm.Field } +// GetPrefix is a convenience function to return the prefix of a requested +// variable for a resource marker. +func (rm *ResourceMarker) GetPrefix() string { + if rm.Field != nil { + return FieldPrefix + } + + return CollectionFieldPrefix +} + // GetSpecPrefix is a convenience function to return the spec prefix of a requested // variable for a resource marker. func (rm *ResourceMarker) GetSpecPrefix() string { @@ -137,6 +147,13 @@ func (rm *ResourceMarker) GetSpecPrefix() string { return CollectionFieldSpecPrefix } +// GetParent is a convenience function to satisfy the MarkerProcessor interface. It will +// always return an empty string for a resource marker because we do not care about parent +// fields. +func (rm *ResourceMarker) GetParent() string { + return "" +} + // Process will process a resource marker from a collection of collection field markers // and field markers, associate them together and set the appropriate fields. func (rm *ResourceMarker) Process(markers *MarkerCollection) error { @@ -242,7 +259,10 @@ func (rm *ResourceMarker) setSourceCode() error { var sourceCodeVar, sourceCodeValue string // get the source code variable - sourceCodeVar = getSourceCodeVariable(rm) + sourceCodeVar, err := getSourceCodeVariable(rm) + if err != nil { + return fmt.Errorf("%w; error retrieving source code variable for resource marker: %s", err, rm) + } // set the source code value and ensure the types match switch value := rm.Value.(type) { diff --git a/internal/workload/v1/markers/resource_marker_internal_test.go b/internal/workload/v1/markers/resource_marker_internal_test.go index e95ee5a..60d1db5 100644 --- a/internal/workload/v1/markers/resource_marker_internal_test.go +++ b/internal/workload/v1/markers/resource_marker_internal_test.go @@ -428,6 +428,8 @@ func TestResourceMarker_isAssociated(t *testing.T) { t.Parallel() randomString := "thisIsRandom" + testMarkerString := "test" + testCollectionString := "test.collection" // this is the test of a standard marker which would be discovered in any manifest type // standalone = no collection involved @@ -437,21 +439,21 @@ func TestResourceMarker_isAssociated(t *testing.T) { // that all field markers on a collection are immediately discovered // as field markers, regardless of how they are labeled. testFieldMarker := &FieldMarker{ - Name: "test", + Name: &testMarkerString, Type: FieldString, forCollection: false, } // this is the test of a standard marker which was discovered on a collection testFieldMarkerOnCollection := &FieldMarker{ - Name: "test.collection", + Name: &testCollectionString, Type: FieldString, forCollection: true, } // this is the test of a standard collection marker. testCollectionMarker := &CollectionFieldMarker{ - Name: "test", + Name: &testMarkerString, Type: FieldString, forCollection: false, } @@ -478,7 +480,7 @@ func TestResourceMarker_isAssociated(t *testing.T) { { name: "operator-builder:resource:field=test is associated with a field marker", fields: fields{ - Field: &testFieldMarker.Name, + Field: testFieldMarker.Name, }, args: args{ fromMarker: testFieldMarker, @@ -488,7 +490,7 @@ func TestResourceMarker_isAssociated(t *testing.T) { { name: "operator-builder:resource:field=test is not associated with a collection field marker", fields: fields{ - Field: &testCollectionMarker.Name, + Field: testCollectionMarker.Name, }, args: args{ fromMarker: testCollectionMarker, @@ -538,7 +540,7 @@ func TestResourceMarker_isAssociated(t *testing.T) { { name: "operator-builder:resource:collectionField=testCollection is associated with a field marker", fields: fields{ - CollectionField: &testCollectionMarker.Name, + CollectionField: testCollectionMarker.Name, }, args: args{ fromMarker: testCollectionMarker, @@ -548,7 +550,7 @@ func TestResourceMarker_isAssociated(t *testing.T) { { name: "operator-builder:resource:collectionField=test is associated with a field marker from a collection", fields: fields{ - CollectionField: &testFieldMarkerOnCollection.Name, + CollectionField: testFieldMarkerOnCollection.Name, }, args: args{ fromMarker: testFieldMarkerOnCollection, @@ -586,27 +588,27 @@ func TestResourceMarker_getFieldMarker(t *testing.T) { testMarkers := &MarkerCollection{ CollectionFieldMarkers: []*CollectionFieldMarker{ { - Name: fieldOne, + Name: &fieldOne, Type: FieldString, }, { - Name: fieldTwo, + Name: &fieldTwo, Type: FieldString, }, }, FieldMarkers: []*FieldMarker{ { - Name: fieldOne, + Name: &fieldOne, Type: FieldString, forCollection: false, }, { - Name: fieldOne, + Name: &fieldOne, Type: FieldString, forCollection: true, }, { - Name: fieldTwo, + Name: &fieldTwo, Type: FieldString, forCollection: false, }, @@ -640,7 +642,7 @@ func TestResourceMarker_getFieldMarker(t *testing.T) { Field: &fieldOne, }, want: &FieldMarker{ - Name: fieldOne, + Name: &fieldOne, Type: FieldString, }, }, @@ -653,7 +655,7 @@ func TestResourceMarker_getFieldMarker(t *testing.T) { CollectionField: &fieldTwo, }, want: &CollectionFieldMarker{ - Name: fieldTwo, + Name: &fieldTwo, Type: FieldString, }, }, @@ -666,7 +668,7 @@ func TestResourceMarker_getFieldMarker(t *testing.T) { Field: &fieldTwo, }, want: &FieldMarker{ - Name: fieldTwo, + Name: &fieldTwo, Type: FieldString, }, }, @@ -679,7 +681,7 @@ func TestResourceMarker_getFieldMarker(t *testing.T) { CollectionField: &fieldOne, }, want: &FieldMarker{ - Name: fieldOne, + Name: &fieldOne, Type: FieldString, forCollection: true, }, @@ -742,23 +744,23 @@ func TestResourceMarker_Process(t *testing.T) { testMarkers := &MarkerCollection{ CollectionFieldMarkers: []*CollectionFieldMarker{ { - Name: fieldOne, + Name: &fieldOne, Type: FieldString, }, }, FieldMarkers: []*FieldMarker{ { - Name: fieldOne, + Name: &fieldOne, Type: FieldString, forCollection: true, }, { - Name: fieldOne, + Name: &fieldOne, Type: FieldString, forCollection: false, }, { - Name: fieldTwo, + Name: &fieldTwo, Type: FieldString, forCollection: true, }, @@ -873,17 +875,17 @@ func TestResourceMarker_setSourceCode(t *testing.T) { testSourceCodeField := "test.nested.field" testCollectionMarker := &CollectionFieldMarker{ - Name: testSourceCodeField, + Name: &testSourceCodeField, Type: FieldString, } testFieldMarker := &FieldMarker{ - Name: testSourceCodeField, + Name: &testSourceCodeField, Type: FieldInt, } testInvalidMarker := &FieldMarker{ - Name: testSourceCodeField, + Name: &testSourceCodeField, Type: FieldUnknownType, } From 0117786782eee05ffd230673517e59932beeaba9 Mon Sep 17 00:00:00 2001 From: Dustin Scott Date: Tue, 14 Jun 2022 07:36:56 -0500 Subject: [PATCH 3/3] chore: fix linter errors Signed-off-by: Dustin Scott --- .../collection_field_marker_internal_test.go | 4 ++-- .../v1/markers/field_marker_internal_test.go | 4 ++-- internal/workload/v1/markers/markers.go | 13 ++++++++++--- .../workload/v1/markers/markers_internal_test.go | 12 ++++++------ 4 files changed, 20 insertions(+), 13 deletions(-) diff --git a/internal/workload/v1/markers/collection_field_marker_internal_test.go b/internal/workload/v1/markers/collection_field_marker_internal_test.go index 3cb3bdc..ecc8b9b 100644 --- a/internal/workload/v1/markers/collection_field_marker_internal_test.go +++ b/internal/workload/v1/markers/collection_field_marker_internal_test.go @@ -35,7 +35,7 @@ func TestCollectionFieldMarker_String(t *testing.T) { Name: &testName, Type: FieldString, Description: &testString, - Default: "test", + Default: testName, }, want: "CollectionFieldMarker{Name: test Type: string Description: \"cfm test\" Default: test}", }, @@ -45,7 +45,7 @@ func TestCollectionFieldMarker_String(t *testing.T) { Name: &testName, Type: FieldString, Description: nil, - Default: "test", + Default: testName, }, want: "CollectionFieldMarker{Name: test Type: string Description: \"\" Default: test}", }, diff --git a/internal/workload/v1/markers/field_marker_internal_test.go b/internal/workload/v1/markers/field_marker_internal_test.go index 471791c..f14c91e 100644 --- a/internal/workload/v1/markers/field_marker_internal_test.go +++ b/internal/workload/v1/markers/field_marker_internal_test.go @@ -35,7 +35,7 @@ func TestFieldMarker_String(t *testing.T) { Name: &testName, Type: FieldString, Description: &testString, - Default: "test", + Default: testName, }, want: "FieldMarker{Name: test Type: string Description: \"fm test\" Default: test}", }, @@ -45,7 +45,7 @@ func TestFieldMarker_String(t *testing.T) { Name: &testName, Type: FieldString, Description: nil, - Default: "test", + Default: testName, }, want: "FieldMarker{Name: test Type: string Description: \"\" Default: test}", }, diff --git a/internal/workload/v1/markers/markers.go b/internal/workload/v1/markers/markers.go index 6f7b05c..7efa8fe 100644 --- a/internal/workload/v1/markers/markers.go +++ b/internal/workload/v1/markers/markers.go @@ -4,6 +4,7 @@ package markers import ( + "errors" "fmt" "regexp" "strings" @@ -14,6 +15,12 @@ import ( markerparser "github.com/vmware-tanzu-labs/operator-builder/internal/markers/marker" ) +var ( + ErrMissingParentOrName = errors.New("missing either parent=value or name=value marker") + ErrInvalidReplaceMarkerFieldType = errors.New("invalid marker type using replace") + ErrInvalidParentField = errors.New("invalid parent field") +) + // MarkerType defines the types of markers that are accepted by the parser. type MarkerType int @@ -145,7 +152,7 @@ func transformYAML(results ...*inspect.YAMLResult) error { // ensure that either a parent or a name is set if marker.GetName() == "" && marker.GetParent() == "" { - return fmt.Errorf("must set either parent=value or name=value for marker %s", marker) + return fmt.Errorf("%w for marker %s", ErrMissingParentOrName, marker) } // get common variables and confirm that we are not working with a reserved marker @@ -219,7 +226,7 @@ func getSourceCodeFieldVariable(marker FieldMarkerProcessor) (string, error) { case FieldInt: return fmt.Sprintf("!!start string(rune(%s)) !!end", marker.GetSourceCodeVariable()), nil default: - return "", fmt.Errorf("cannot handle field type: %s", marker.GetFieldType()) + return "", fmt.Errorf("%w with field type %s", ErrInvalidReplaceMarkerFieldType, marker.GetFieldType()) } } @@ -234,7 +241,7 @@ func getSourceCodeVariable(marker MarkerProcessor) (string, error) { return fmt.Sprintf("%s.%s", marker.GetPrefix(), supportedParents()[marker.GetParent()]), nil } - return "", fmt.Errorf("parent field requested %s but is not supported. supported parent fields are: %v", marker.GetParent(), supportedParents()) + return "", fmt.Errorf("%w %s. supported parent fields are: %v", ErrInvalidParentField, marker.GetParent(), supportedParents()) } // getKeyValue gets the key and value from a YAML result. diff --git a/internal/workload/v1/markers/markers_internal_test.go b/internal/workload/v1/markers/markers_internal_test.go index 7c49325..3402df9 100644 --- a/internal/workload/v1/markers/markers_internal_test.go +++ b/internal/workload/v1/markers/markers_internal_test.go @@ -510,7 +510,7 @@ func Test_setValue(t *testing.T) { //nolint: goconst testInvalidReplaceText := "*&^%" testReplaceText := "" - testField := "test.field" + testField := "test.field.set" type args struct { marker FieldMarkerProcessor @@ -528,7 +528,7 @@ func Test_setValue(t *testing.T) { args: args{ marker: &FieldMarker{ Name: &testField, - sourceCodeVar: "parent.Spec.Test.Field", + sourceCodeVar: "parent.Spec.Test.Field.Set", Type: FieldString, }, value: &yaml.Node{ @@ -539,7 +539,7 @@ func Test_setValue(t *testing.T) { wantErr: false, want: &yaml.Node{ Tag: "!!var", - Value: "parent.Spec.Test.Field", + Value: "parent.Spec.Test.Field.Set", }, }, { @@ -548,7 +548,7 @@ func Test_setValue(t *testing.T) { marker: &FieldMarker{ Name: &testField, Replace: &testReplaceText, - sourceCodeVar: "parent.Spec.Test.Field", + sourceCodeVar: "parent.Spec.Test.Field.Set", Type: FieldString, }, value: &yaml.Node{ @@ -559,7 +559,7 @@ func Test_setValue(t *testing.T) { wantErr: false, want: &yaml.Node{ Tag: "!!str", - Value: "test !!start parent.Spec.Test.Field !!end value", + Value: "test !!start parent.Spec.Test.Field.Set !!end value", }, }, { @@ -568,7 +568,7 @@ func Test_setValue(t *testing.T) { marker: &FieldMarker{ Name: &testField, Replace: &testInvalidReplaceText, - sourceCodeVar: "parent.Spec.Test.Field", + sourceCodeVar: "parent.Spec.Test.Field.Set", Type: FieldString, }, value: &yaml.Node{