Remove unnecessary code

- Use opendev.org/airship/airshipctl/pkg/document.Selector
   instead of sigs.k8s.io/kustomize/api/types.Selector
   which makes selectors conversion code unnecessary

 - Use document.GetSecretDataKey instead of
   document.DecodeSecretData which removes code duplicates

Change-Id: Ie2c6b8d8222b7acb1b657f8d786a8c3a06b0c6fd

pkg/api/v1alpha1/isoconfiguration.go

@@ -16,7 +16,8 @@ package v1alpha1
 
 import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"sigs.k8s.io/kustomize/api/types"
+
+	"opendev.org/airship/airshipctl/pkg/document"
 )
 
 // IsoContainer structure contains parameters related to Docker runtime, used for building image
@@ -32,11 +33,11 @@ type IsoContainer struct {
 // Isogen structure defines document selection criteria for cloud-init metadata
 type Isogen struct {
 	// Cloud Init user data will be retrieved from the doc matching this object
-	UserDataSelector types.Selector `json:"userDataSelector,omitempty"`
+	UserDataSelector document.Selector `json:"userDataSelector,omitempty"`
 	// Cloud init user data will be retrieved from this document key
 	UserDataKey string `json:"userDataKey,omitempty"`
 	// Cloud Init network config will be retrieved from the doc matching this object
-	NetworkConfigSelector types.Selector `json:"networkConfigSelector,omitempty"`
+	NetworkConfigSelector document.Selector `json:"networkConfigSelector,omitempty"`
 	// Cloud init network config will be retrieved from this document key
 	NetworkConfigKey string `json:"networkConfigKey,omitempty"`
 	// File name to use for the output image that will be written to the container volume root

pkg/bootstrap/cloudinit/cloud-init.go

@@ -16,51 +16,40 @@ package cloudinit
 
 import (
 	"opendev.org/airship/airshipctl/pkg/document"
-
-	"sigs.k8s.io/kustomize/api/resid"
-	"sigs.k8s.io/kustomize/api/types"
 )
 
-var (
+const (
-	// Initialize defaults where we expect to find user-data and
+	defaultUserDataKey      = "userData"
-	// network config data in manifests
+	defaultNetworkConfigKey = "networkData"
-	userDataSelectorDefaults = types.Selector{
-		Gvk:           resid.Gvk{Kind: document.SecretKind},
-		LabelSelector: document.EphemeralUserDataSelector,
-	}
-	userDataKeyDefault            = "userData"
-	networkConfigSelectorDefaults = types.Selector{
-		Gvk:           resid.Gvk{Kind: document.BareMetalHostKind},
-		LabelSelector: document.EphemeralHostSelector,
-	}
-	networkConfigKeyDefault = "networkData"
 )
 
 // GetCloudData reads YAML document input and generates cloud-init data for
 // ephemeral node.
 func GetCloudData(
 	docBundle document.Bundle,
-	userDataSelector types.Selector,
+	userDataSelector document.Selector,
 	userDataKey string,
-	networkConfigSelector types.Selector,
+	networkConfigSelector document.Selector,
 	networkConfigKey string,
 ) (userData []byte, netConf []byte, err error) {
 	userDataSelectorFinal, userDataKeyFinal := applyDefaultsAndGetData(
 		userDataSelector,
-		userDataSelectorDefaults,
+		document.SecretKind,
+		document.EphemeralUserDataSelector,
 		userDataKey,
-		userDataKeyDefault,
+		defaultUserDataKey,
 	)
-	userData, err = document.GetSecretData(docBundle, userDataSelectorFinal, userDataKeyFinal)
+	userData, err = getUserData(docBundle, userDataSelectorFinal, userDataKeyFinal)
 	if err != nil {
 		return nil, nil, err
 	}
 
 	netConfSelectorFinal, netConfKeyFinal := applyDefaultsAndGetData(
 		networkConfigSelector,
-		networkConfigSelectorDefaults,
+		document.BareMetalHostKind,
+		document.EphemeralHostSelector,
 		networkConfigKey,
-		networkConfigKeyDefault,
+		defaultNetworkConfigKey,
 	)
 	netConf, err = getNetworkData(docBundle, netConfSelectorFinal, netConfKeyFinal)
 	if err != nil {
@@ -71,18 +60,18 @@ func GetCloudData(
 }
 
 func applyDefaultsAndGetData(
-	docSelector types.Selector,
+	docSelector document.Selector,
-	docSelectorDefaults types.Selector,
+	defaultKind string,
+	defaultLabel string,
 	key string,
 	keyDefault string,
-) (types.Selector, string) {
+) (document.Selector, string) {
 	// Assign defaults if there are no user supplied overrides
 	if docSelector.Kind == "" &&
 		docSelector.Name == "" &&
 		docSelector.AnnotationSelector == "" &&
 		docSelector.LabelSelector == "" {
-		docSelector.Kind = docSelectorDefaults.Kind
+		docSelector = docSelector.ByKind(defaultKind).ByLabel(defaultLabel)
-		docSelector.LabelSelector = docSelectorDefaults.LabelSelector
 	}
 
 	keyFinal := key
@@ -93,20 +82,37 @@ func applyDefaultsAndGetData(
 	return docSelector, keyFinal
 }
 
+func getUserData(
+	docBundle document.Bundle,
+	userDataSelector document.Selector,
+	userDataKey string,
+) ([]byte, error) {
+	doc, err := docBundle.SelectOne(userDataSelector)
+	if err != nil {
+		return nil, err
+	}
+
+	data, err := document.GetSecretDataKey(doc, userDataKey)
+	if err != nil {
+		return nil, err
+	}
+
+	return []byte(data), nil
+}
+
 func getNetworkData(
 	docBundle document.Bundle,
-	netCfgSelector types.Selector,
+	netCfgSelector document.Selector,
 	netCfgKey string,
 ) ([]byte, error) {
 	// find the baremetal host indicated as the ephemeral node
-	selector := document.NewSelector().ByKind(netCfgSelector.Kind).ByLabel(netCfgSelector.LabelSelector)
+	d, err := docBundle.SelectOne(netCfgSelector)
-	d, err := docBundle.SelectOne(selector)
 	if err != nil {
 		return nil, err
 	}
 
 	// try and find these documents in our bundle
-	selector, err = document.NewNetworkDataSelector(d)
+	selector, err := document.NewNetworkDataSelector(d)
 	if err != nil {
 		return nil, err
 	}
@@ -117,10 +123,10 @@ func getNetworkData(
 	}
 
 	// finally, try and retrieve the data we want from the document
-	netData, err := document.DecodeSecretData(d, netCfgKey)
+	netData, err := document.GetSecretDataKey(d, netCfgKey)
 	if err != nil {
 		return nil, err
 	}
 
-	return netData, nil
+	return []byte(netData), nil
 }

pkg/bootstrap/cloudinit/cloud-init_test.go

@@ -19,9 +19,30 @@ import (
 	"github.com/stretchr/testify/require"
 
 	"opendev.org/airship/airshipctl/pkg/document"
+)
 
-	"sigs.k8s.io/kustomize/api/resid"
+type selectors struct {
-	"sigs.k8s.io/kustomize/api/types"
+	userDataSelector      document.Selector
+	userDataKey           string
+	networkConfigSelector document.Selector
+	networkConfigKey      string
+}
+
+var (
+	emptySelectors = selectors{
+		userDataSelector:      document.NewSelector(),
+		networkConfigSelector: document.NewSelector(),
+	}
+	validSelectors = selectors{
+		userDataSelector: document.NewSelector().
+			ByKind("Secret").
+			ByLabel("airshipit.org/ephemeral-user-data in (True, true)"),
+		userDataKey: defaultUserDataKey,
+		networkConfigSelector: document.NewSelector().
+			ByKind("BareMetalHost").
+			ByLabel("airshipit.org/ephemeral-node in (True, true)"),
+		networkConfigKey: defaultNetworkConfigKey,
+	}
 )
 
 func TestGetCloudData(t *testing.T) {
@@ -29,35 +50,25 @@ func TestGetCloudData(t *testing.T) {
 	require.NoError(t, err, "Building Bundle Failed")
 
 	tests := []struct {
-		labelFilter           string
+		name string
-		userDataSelector      types.Selector
+		selectors
-		userDataKey           string
+		labelFilter      string
-		networkConfigSelector types.Selector
+		expectedUserData []byte
-		networkConfigKey      string
+		expectedNetData  []byte
-		expectedUserData      []byte
+		expectedErr      error
-		expectedNetData       []byte
-		expectedErr           error
 	}{
 		{
-			labelFilter:           "test=validdocset",
+			name:             "Default selectors",
-			userDataSelector:      types.Selector{},
+			labelFilter:      "test=validdocset",
-			networkConfigSelector: types.Selector{},
+			selectors:        emptySelectors,
-			expectedUserData:      []byte("cloud-init"),
+			expectedUserData: []byte("cloud-init"),
-			expectedNetData:       []byte("net-config"),
+			expectedNetData:  []byte("net-config"),
-			expectedErr:           nil,
+			expectedErr:      nil,
 		},
 		{
-			labelFilter: "test=ephemeralmissing",
+			name:             "BareMetalHost document not found",
-			userDataSelector: types.Selector{
+			labelFilter:      "test=ephemeralmissing",
-				Gvk:           resid.Gvk{Kind: "Secret"},
+			selectors:        validSelectors,
-				LabelSelector: "airshipit.org/ephemeral-user-data in (True, true)",
-			},
-			userDataKey: "userData",
-			networkConfigSelector: types.Selector{
-				Gvk:           resid.Gvk{Kind: "BareMetalHost"},
-				LabelSelector: "airshipit.org/ephemeral-node in (True, true)",
-			},
-			networkConfigKey: "networkData",
 			expectedUserData: nil,
 			expectedNetData:  nil,
 			expectedErr: document.ErrDocNotFound{
@@ -67,17 +78,9 @@ func TestGetCloudData(t *testing.T) {
 			},
 		},
 		{
-			labelFilter: "test=ephemeralduplicate",
+			name:             "BareMetalHost document duplication",
-			userDataSelector: types.Selector{
+			labelFilter:      "test=ephemeralduplicate",
-				Gvk:           resid.Gvk{Kind: "Secret"},
+			selectors:        validSelectors,
-				LabelSelector: "airshipit.org/ephemeral-user-data in (True, true)",
-			},
-			userDataKey: "userData",
-			networkConfigSelector: types.Selector{
-				Gvk:           resid.Gvk{Kind: "BareMetalHost"},
-				LabelSelector: "airshipit.org/ephemeral-node in (True, true)",
-			},
-			networkConfigKey: "networkData",
 			expectedUserData: nil,
 			expectedNetData:  nil,
 			expectedErr: document.ErrMultiDocsFound{
@@ -87,17 +90,9 @@ func TestGetCloudData(t *testing.T) {
 			},
 		},
 		{
-			labelFilter: "test=networkdatabadpointer",
+			name:             "Bad network data document reference",
-			userDataSelector: types.Selector{
+			labelFilter:      "test=networkdatabadpointer",
-				Gvk:           resid.Gvk{Kind: "Secret"},
+			selectors:        validSelectors,
-				LabelSelector: "airshipit.org/ephemeral-user-data in (True, true)",
-			},
-			userDataKey: "userData",
-			networkConfigSelector: types.Selector{
-				Gvk:           resid.Gvk{Kind: "BareMetalHost"},
-				LabelSelector: "airshipit.org/ephemeral-node in (True, true)",
-			},
-			networkConfigKey: "networkData",
 			expectedUserData: nil,
 			expectedNetData:  nil,
 			expectedErr: document.ErrDocNotFound{
@@ -108,55 +103,31 @@ func TestGetCloudData(t *testing.T) {
 			},
 		},
 		{
-			labelFilter: "test=networkdatamalformed",
+			name:             "Bad network data document structure",
-			userDataSelector: types.Selector{
+			labelFilter:      "test=networkdatamalformed",
-				Gvk:           resid.Gvk{Kind: "Secret"},
+			selectors:        validSelectors,
-				LabelSelector: "airshipit.org/ephemeral-user-data in (True, true)",
-			},
-			userDataKey: "userData",
-			networkConfigSelector: types.Selector{
-				Gvk:           resid.Gvk{Kind: "BareMetalHost"},
-				LabelSelector: "airshipit.org/ephemeral-node in (True, true)",
-			},
-			networkConfigKey: "networkData",
 			expectedUserData: nil,
 			expectedNetData:  nil,
-			expectedErr: document.ErrDataNotSupplied{
+			expectedErr: document.ErrDocumentDataKeyNotFound{
 				DocName: "networkdatamalformed-malformed",
-				Key:     networkConfigKeyDefault,
+				Key:     defaultNetworkConfigKey,
 			},
 		},
 		{
-			labelFilter: "test=userdatamalformed",
+			name:             "Bad user data document structure",
-			userDataSelector: types.Selector{
+			labelFilter:      "test=userdatamalformed",
-				Gvk:           resid.Gvk{Kind: "Secret"},
+			selectors:        validSelectors,
-				LabelSelector: "airshipit.org/ephemeral-user-data in (True, true)",
-			},
-			userDataKey: "userData",
-			networkConfigSelector: types.Selector{
-				Gvk:           resid.Gvk{Kind: "BareMetalHost"},
-				LabelSelector: "airshipit.org/ephemeral-node in (True, true)",
-			},
-			networkConfigKey: "networkData",
 			expectedUserData: nil,
 			expectedNetData:  nil,
-			expectedErr: document.ErrDataNotSupplied{
+			expectedErr: document.ErrDocumentDataKeyNotFound{
 				DocName: "userdatamalformed-somesecret",
-				Key:     userDataKeyDefault,
+				Key:     defaultUserDataKey,
 			},
 		},
 		{
-			labelFilter: "test=userdatamissing",
+			name:             "User data document not found",
-			userDataSelector: types.Selector{
+			labelFilter:      "test=userdatamissing",
-				Gvk:           resid.Gvk{Kind: "Secret"},
+			selectors:        validSelectors,
-				LabelSelector: "airshipit.org/ephemeral-user-data in (True, true)",
-			},
-			userDataKey: "userData",
-			networkConfigSelector: types.Selector{
-				Gvk:           resid.Gvk{Kind: "BareMetalHost"},
-				LabelSelector: "airshipit.org/ephemeral-node in (True, true)",
-			},
-			networkConfigKey: "networkData",
 			expectedUserData: nil,
 			expectedNetData:  nil,
 			expectedErr: document.ErrDocNotFound{
@@ -168,26 +139,28 @@ func TestGetCloudData(t *testing.T) {
 	}
 
 	for _, tt := range tests {
-		// prune the bundle down using the label filter for the specific test
+		t.Run(tt.name, func(t *testing.T) {
-		selector := document.NewSelector().ByLabel(tt.labelFilter)
+			// prune the bundle down using the label filter for the specific test
-		filteredBundle, err := bundle.SelectBundle(selector)
+			selector := document.NewSelector().ByLabel(tt.labelFilter)
-		require.NoError(t, err, "Building filtered bundle for %s failed", tt.labelFilter)
+			filteredBundle, err := bundle.SelectBundle(selector)
+			require.NoError(t, err, "Building filtered bundle for %s failed", tt.labelFilter)
 
-		// ensure each test case filter has at least one document
+			// ensure each test case filter has at least one document
-		docs, err := filteredBundle.GetAllDocuments()
+			docs, err := filteredBundle.GetAllDocuments()
-		require.NoError(t, err, "GetAllDocuments failed")
+			require.NoError(t, err, "GetAllDocuments failed")
-		require.NotZero(t, docs)
+			require.NotZero(t, docs)
 
-		actualUserData, actualNetData, actualErr := GetCloudData(
+			actualUserData, actualNetData, actualErr := GetCloudData(
-			filteredBundle,
+				filteredBundle,
-			tt.userDataSelector,
+				tt.userDataSelector,
-			tt.userDataKey,
+				tt.userDataKey,
-			tt.networkConfigSelector,
+				tt.networkConfigSelector,
-			tt.networkConfigKey,
+				tt.networkConfigKey,
-		)
+			)
 
-		assert.Equal(t, tt.expectedUserData, actualUserData)
+			assert.Equal(t, tt.expectedUserData, actualUserData)
-		assert.Equal(t, tt.expectedNetData, actualNetData)
+			assert.Equal(t, tt.expectedNetData, actualNetData)
-		assert.Equal(t, tt.expectedErr, actualErr)
+			assert.Equal(t, tt.expectedErr, actualErr)
+		})
 	}
 }

pkg/document/document.go

@@ -15,7 +15,6 @@
 package document
 
 import (
-	b64 "encoding/base64"
 	"fmt"
 
 	"k8s.io/apimachinery/pkg/runtime"
@@ -298,31 +297,3 @@ func NewDocumentFromBytes(b []byte) (Document, error) {
 	err = doc.SetKustomizeResource(res)
 	return doc, err
 }
-
-// DecodeSecretData returns base64-decoded secret data
-func DecodeSecretData(cfg Document, key string) ([]byte, error) {
-	var needsBase64Decode = false
-
-	// TODO(alanmeadows): distinguish between missing key
-	// and missing data/stringData keys in the Secret
-	data, err := cfg.GetStringMap("data")
-	if err == nil {
-		needsBase64Decode = true
-	} else {
-		// we'll catch any error below
-		data, err = cfg.GetStringMap("stringData")
-		if err != nil {
-			return nil, ErrDataNotSupplied{DocName: cfg.GetName(), Key: "data or stringData"}
-		}
-	}
-
-	res, ok := data[key]
-	if !ok {
-		return nil, ErrDataNotSupplied{DocName: cfg.GetName(), Key: key}
-	}
-
-	if needsBase64Decode {
-		return b64.StdEncoding.DecodeString(res)
-	}
-	return []byte(res), nil
-}

pkg/document/errors.go

@@ -49,19 +49,6 @@ type ErrRuntimeObjectKind struct {
 	Obj runtime.Object
 }
 
-// ErrDataNotSupplied error returned of no data in the Secret
-type ErrDataNotSupplied struct {
-	DocName string
-	Key     string
-}
-
-// ErrDuplicateNetworkDataDocuments error returned if multiple matching documents
-// were found with the same name in the same namespace
-type ErrDuplicateNetworkDataDocuments struct {
-	DocName   string
-	Namespace string
-}
-
 // ErrBadValueFormat returned if wrong field type requested
 type ErrBadValueFormat struct {
 	Value    string
@@ -89,14 +76,6 @@ func (e ErrRuntimeObjectKind) Error() string {
 	return fmt.Sprintf("object %#v has either none or multiple kinds in scheme (expected one)", e.Obj)
 }
 
-func (e ErrDataNotSupplied) Error() string {
-	return fmt.Sprintf("Document %s has no key %s", e.DocName, e.Key)
-}
-
-func (e ErrDuplicateNetworkDataDocuments) Error() string {
-	return fmt.Sprintf("Found more than one document with the name %s in namespace %s", e.DocName, e.Namespace)
-}
-
 func (e ErrBadValueFormat) Error() string {
 	return fmt.Sprintf("value of %s expected to have %s type, got %s", e.Value, e.Expected, e.Actual)
 }

pkg/document/selectors.go

@@ -197,37 +197,3 @@ func NewValidatorExecutorSelector() Selector {
 		DocumentValidationKind).
 		ByName(DocumentValidationName)
 }
-
-//GetSecretData returns data located with a given key of a given document
-func GetSecretData(docBundle Bundle, apiSelector types.Selector, key string) ([]byte, error) {
-	s := NewSelector()
-	if apiSelector.Group != "" && apiSelector.Version != "" && apiSelector.Kind != "" {
-		s = s.ByGvk(apiSelector.Group, apiSelector.Version, apiSelector.Kind)
-	} else if apiSelector.Kind != "" {
-		s = s.ByKind(apiSelector.Kind)
-	}
-	if apiSelector.Namespace != "" {
-		s = s.ByNamespace(apiSelector.Namespace)
-	}
-	if apiSelector.Name != "" {
-		s = s.ByName(apiSelector.Name)
-	}
-	if apiSelector.AnnotationSelector != "" {
-		s = s.ByAnnotation(apiSelector.AnnotationSelector)
-	}
-	if apiSelector.LabelSelector != "" {
-		s = s.ByLabel(apiSelector.LabelSelector)
-	}
-
-	doc, docErr := docBundle.SelectOne(s)
-	if docErr != nil {
-		return nil, docErr
-	}
-
-	// finally, try and retrieve the data we want from the document
-	data, keyErr := DecodeSecretData(doc, key)
-	if keyErr != nil {
-		return nil, keyErr
-	}
-	return data, nil
-}

pkg/document/selectors_test.go

@@ -60,16 +60,6 @@ func TestSelectorsPositive(t *testing.T) {
 		require.NoError(t, err)
 		assert.Len(t, doc, 1)
 	})
-
-	t.Run("TestGetSecretData", func(t *testing.T) {
-		data, err := document.GetSecretData(bundle, types.Selector{
-			Gvk:           resid.Gvk{Kind: "Secret"},
-			LabelSelector: "airshipit.org/ephemeral-user-data",
-		},
-			"userData")
-		require.NoError(t, err)
-		assert.Equal(t, []byte("cloud-init"), data)
-	})
 }
 
 func TestSelectorsNegative(t *testing.T) {