Skip to content

Commit

Permalink
Remove strings.TrimSpace()
Browse files Browse the repository at this point in the history
Names and labels cannot have any whitespace. If they do, it's an error that should be reported and fixed.
  • Loading branch information
ash2k committed Aug 8, 2021
1 parent 69388f2 commit 4b8dd62
Show file tree
Hide file tree
Showing 8 changed files with 18 additions and 79 deletions.
8 changes: 4 additions & 4 deletions cmd/printers/events/formatter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ func TestFormatter_FormatApplyEvent(t *testing.T) {
err := formatter.FormatApplyEvent(tc.event)
assert.NoError(t, err)

assert.Equal(t, strings.TrimSpace(tc.expected), strings.TrimSpace(out.String()))
assert.Equal(t, tc.expected, strings.TrimSpace(out.String()))
})
}
}
Expand Down Expand Up @@ -116,7 +116,7 @@ func TestFormatter_FormatStatusEvent(t *testing.T) {
err := formatter.FormatStatusEvent(tc.event)
assert.NoError(t, err)

assert.Equal(t, strings.TrimSpace(tc.expected), strings.TrimSpace(out.String()))
assert.Equal(t, tc.expected, strings.TrimSpace(out.String()))
})
}
}
Expand Down Expand Up @@ -161,7 +161,7 @@ func TestFormatter_FormatPruneEvent(t *testing.T) {
err := formatter.FormatPruneEvent(tc.event)
assert.NoError(t, err)

assert.Equal(t, strings.TrimSpace(tc.expected), strings.TrimSpace(out.String()))
assert.Equal(t, tc.expected, strings.TrimSpace(out.String()))
})
}
}
Expand Down Expand Up @@ -210,7 +210,7 @@ func TestFormatter_FormatDeleteEvent(t *testing.T) {
err := formatter.FormatDeleteEvent(tc.event)
assert.NoError(t, err)

assert.Equal(t, strings.TrimSpace(tc.expected), strings.TrimSpace(out.String()))
assert.Equal(t, tc.expected, strings.TrimSpace(out.String()))
})
}
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/apply/applier.go
Original file line number Diff line number Diff line change
Expand Up @@ -270,12 +270,12 @@ func handleError(eventChannel chan event.Event, err error) {
func localNamespaces(localInv inventory.InventoryInfo, localObjs []object.ObjMetadata) sets.String {
namespaces := sets.NewString()
for _, obj := range localObjs {
namespace := strings.TrimSpace(strings.ToLower(obj.Namespace))
namespace := strings.ToLower(obj.Namespace)
if namespace != "" {
namespaces.Insert(namespace)
}
}
invNamespace := strings.TrimSpace(strings.ToLower(localInv.Namespace()))
invNamespace := strings.ToLower(localInv.Namespace())
if invNamespace != "" {
namespaces.Insert(invNamespace)
}
Expand Down
4 changes: 1 addition & 3 deletions pkg/apply/taskrunner/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@
package taskrunner

import (
"strings"

"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/sets"
"sigs.k8s.io/cli-utils/pkg/apply/event"
Expand Down Expand Up @@ -81,7 +79,7 @@ func (tc *TaskContext) AppliedResources() []object.ObjMetadata {
func (tc *TaskContext) AppliedResourceUIDs() sets.String {
uids := sets.NewString()
for _, ai := range tc.appliedResources {
uid := strings.TrimSpace(string(ai.uid))
uid := string(ai.uid)
if uid != "" {
uids.Insert(uid)
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/inventory/inventory.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ func retrieveInventoryLabel(obj *unstructured.Unstructured) (string, error) {
if !exists {
return "", fmt.Errorf("inventory label does not exist for inventory object: %s", common.InventoryLabel)
}
return strings.TrimSpace(inventoryLabel), nil
return inventoryLabel, nil
}

// ValidateNoInventory takes a slice of unstructured.Unstructured objects and
Expand Down
29 changes: 0 additions & 29 deletions pkg/inventory/inventory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,29 +124,6 @@ var pod3Info = &resource.Info{
Object: pod3,
}

var inventoryObjLabelWithSpace = unstructured.Unstructured{
Object: map[string]interface{}{
"apiVersion": "v1",
"kind": "ConfigMap",
"metadata": map[string]interface{}{
"name": inventoryObjName,
"namespace": testNamespace,
"labels": map[string]interface{}{
common.InventoryLabel: "\tinventory-label ",
},
},
},
}

var invInfoLabelWithSpace = &resource.Info{
Namespace: testNamespace,
Name: inventoryObjName,
Mapping: &meta.RESTMapping{
Scope: meta.RESTScopeNamespace,
},
Object: &inventoryObjLabelWithSpace,
}

func TestFindInventoryObj(t *testing.T) {
tests := map[string]struct {
infos []*unstructured.Unstructured
Expand Down Expand Up @@ -239,12 +216,6 @@ func TestRetrieveInventoryLabel(t *testing.T) {
inventoryLabel: "",
isError: true,
},
// Retrieves label without preceding/trailing whitespace.
{
inventoryInfo: invInfoLabelWithSpace,
inventoryLabel: "inventory-label",
isError: false,
},
{
inventoryInfo: invInfo,
inventoryLabel: testInventoryLabel,
Expand Down
12 changes: 2 additions & 10 deletions pkg/inventory/inventorycm.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ package inventory

import (
"fmt"
"strings"

"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"sigs.k8s.io/cli-utils/pkg/common"
Expand Down Expand Up @@ -59,15 +58,8 @@ func (icm *InventoryConfigMap) Namespace() string {
}

func (icm *InventoryConfigMap) ID() string {
labels := icm.inv.GetLabels()
if len(labels) == 0 {
return ""
}
inventoryLabel, exists := labels[common.InventoryLabel]
if !exists {
return ""
}
return strings.TrimSpace(inventoryLabel)
// Empty string if not set.
return icm.inv.GetLabels()[common.InventoryLabel]
}

func (icm *InventoryConfigMap) Strategy() InventoryStrategy {
Expand Down
7 changes: 3 additions & 4 deletions pkg/object/objmetadata.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,15 +78,14 @@ func (oms ObjMetas) Contains(id ObjMetadata) bool {
// passed fields and returns an error for bad parameters.
func CreateObjMetadata(namespace string, name string, gk schema.GroupKind) (ObjMetadata, error) {
// Namespace can be empty, but name cannot.
name = strings.TrimSpace(name)
if name == "" {
return NilObjMetadata, fmt.Errorf("empty name for object")
}
if gk.Kind == "" {
return NilObjMetadata, fmt.Errorf("empty kind for object")
}
return ObjMetadata{
Namespace: strings.TrimSpace(namespace),
Namespace: namespace,
Name: name,
GroupKind: gk,
}, nil
Expand Down Expand Up @@ -133,8 +132,8 @@ func ParseObjMetadata(s string) (ObjMetadata, error) {
}
// Create the ObjMetadata object from the four parsed fields.
gk := schema.GroupKind{
Group: strings.TrimSpace(group),
Kind: strings.TrimSpace(kind),
Group: group,
Kind: kind,
}
return CreateObjMetadata(namespace, name, gk)
}
Expand Down
31 changes: 5 additions & 26 deletions pkg/object/objmetadata_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
package object

import (
"strings"
"testing"

rbacv1 "k8s.io/api/rbac/v1"
Expand All @@ -19,29 +18,9 @@ func TestCreateObjMetadata(t *testing.T) {
expected string
isError bool
}{
"Namespace with only whitespace": {
namespace: " \n",
name: " test-name\t",
gk: schema.GroupKind{
Group: "apps",
Kind: "ReplicaSet",
},
expected: "_test-name_apps_ReplicaSet",
isError: false,
},
"Name with leading/trailing whitespace": {
namespace: "test-namespace ",
name: " test-name\t",
gk: schema.GroupKind{
Group: "apps",
Kind: "ReplicaSet",
},
expected: "test-namespace_test-name_apps_ReplicaSet",
isError: false,
},
"Empty name is an error": {
namespace: "test-namespace ",
name: " \t",
name: "",
gk: schema.GroupKind{
Group: "apps",
Kind: "ReplicaSet",
Expand Down Expand Up @@ -81,8 +60,8 @@ func TestCreateObjMetadata(t *testing.T) {
// so that tests will catch any change to CreateObjMetadata that
// would break ParseObjMetadata.
expectedObjMetadata := &ObjMetadata{
Namespace: strings.TrimSpace(tc.namespace),
Name: strings.TrimSpace(tc.name),
Namespace: tc.namespace,
Name: tc.name,
GroupKind: tc.gk,
}
actual, err := ParseObjMetadata(inv.String())
Expand Down Expand Up @@ -189,8 +168,8 @@ func TestParseObjMetadata(t *testing.T) {
inventory *ObjMetadata
isError bool
}{
"Simple inventory string parse with empty namespace and whitespace": {
invStr: "_test-name_apps_ReplicaSet\t",
"Simple inventory string parse with empty namespace": {
invStr: "_test-name_apps_ReplicaSet",
inventory: &ObjMetadata{
Name: "test-name",
GroupKind: schema.GroupKind{
Expand Down

0 comments on commit 4b8dd62

Please sign in to comment.