Add optional key to the FieldExport target (#100)

FieldExport generates the key name with an output structure like
`<namespace>.<FieldExport-resource-name>`. This default naming structure
creates non-conflicting names as the keys. However, many applications
expect short names as keys. This is important, especially when the
Secret/ConfigMap is mounted as files; the file names are nothing but
the key names.

This PR adds an optional attribute (.spec.to.key) to specify the key
name in the FieldExport resource. If this attribute is specified,
use this key name instead of the default.

Signed-off-by: Baiju Muthukadan <[email protected]>

Issue #, if available: aws-controllers-k8s/community#1410

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Author: baijum

apis/core/v1alpha1/field_export.go

@@ -24,6 +24,8 @@ type FieldExportTarget struct {
 	// Namespace is marked as optional, so we cannot compose `NamespacedName`
 	Namespace *string               `json:"namespace,omitempty"`
 	Kind      FieldExportOutputType `json:"kind"`
+	// Key overrides the default value (`<namespace>.<FieldExport-resource-name>`) for the FieldExport target
+	Key *string `json:"key,omitempty"`
 }
 
 // FieldExportSpec defines the desired state of the FieldExport.

apis/core/v1alpha1/zz_generated.deepcopy.go

@@ -66,6 +66,10 @@ spec:
                 description: FieldExportTarget provides the values necessary to identify
                   the output path for a field export.
                 properties:
+                  key:
+                    description: Key overrides the default value (`<namespace>.<FieldExport-resource-name>`)
+                      for the FieldExport target
+                    type: string
                   kind:
                     description: FieldExportOutputType represents all types that can
                       be produced by a field export operation

config/crd/bases/services.k8s.aws_fieldexports.yaml

@@ -296,6 +296,9 @@ func (r *fieldExportReconciler) writeToConfigMap(
 ) error {
 	// Construct the data key
 	key := fmt.Sprintf("%s.%s", desired.Namespace, desired.Name)
+	if desired.Spec.To != nil && desired.Spec.To.Key != nil && strings.TrimSpace(*desired.Spec.To.Key) != "" {
+		key = *desired.Spec.To.Key
+	}
 
 	// Get the initial configmap
 	nsn := types.NamespacedName{
@@ -340,6 +343,9 @@ func (r *fieldExportReconciler) writeToSecret(
 ) error {
 	// Construct the data key
 	key := fmt.Sprintf("%s.%s", desired.Namespace, desired.Name)
+	if desired.Spec.To != nil && desired.Spec.To.Key != nil && strings.TrimSpace(*desired.Spec.To.Key) != "" {
+		key = *desired.Spec.To.Key
+	}
 
 	// Get the initial secret
 	nsn := types.NamespacedName{

pkg/runtime/field_export_reconciler.go

@@ -163,6 +163,36 @@ func fieldExportWithPath(namespace, name string, kind ackv1alpha1.FieldExportOut
 	}
 }
 
+func fieldExportWithKey(namespace, name string, kind ackv1alpha1.FieldExportOutputType, key string) *ackv1alpha1.FieldExport {
+	path := ".spec.name"
+	return &ackv1alpha1.FieldExport{
+		TypeMeta: v1.TypeMeta{},
+		ObjectMeta: v1.ObjectMeta{
+			Namespace:  namespace,
+			Name:       name,
+			Finalizers: []string{"finalizers.services.k8s.aws/FieldExport"},
+		},
+		Spec: ackv1alpha1.FieldExportSpec{
+			From: &ackv1alpha1.ResourceFieldSelector{
+				Path: &path,
+				Resource: ackv1alpha1.NamespacedResource{
+					GroupKind: v1.GroupKind{
+						Group: BookGVK.Group,
+						Kind:  BookGVK.Kind,
+					},
+					Name: strPtr(SourceResourceName),
+				},
+			},
+			To: &ackv1alpha1.FieldExportTarget{
+				Name: strPtr("fake-export-output"),
+				Kind: kind,
+				Key:  &key,
+			},
+		},
+		Status: ackv1alpha1.FieldExportStatus{},
+	}
+}
+
 func mockFieldExportList() []ackv1alpha1.FieldExport {
 	// Matching cases
 	defaultConfigMap := fieldExportConfigMap(FieldExportNamespace, "export-1")
@@ -452,6 +482,66 @@ func TestSync_HappyCaseResourceNoExports(t *testing.T) {
 	require.Len(exports, 0)
 }
 
+func TestSync_SetKeyNameExplicitly(t *testing.T) {
+	// Setup
+	require := require.New(t)
+	// Mock resource creation
+	r, kc, apiReader := mockFieldExportReconciler()
+	descriptor, res, _ := mockDescriptorAndAWSResource()
+	manager := mockManager()
+	fieldExport := fieldExportWithKey(FieldExportNamespace, FieldExportName, ackv1alpha1.FieldExportOutputTypeSecret, "new-key")
+	sourceResource, _, _ := mockSourceResource()
+	ctx := context.TODO()
+	statusWriter := &ctrlrtclientmock.StatusWriter{}
+
+	//Mock behavior setup
+	setupMockClientForFieldExport(kc, statusWriter, ctx, fieldExport)
+	setupMockApiReaderForFieldExport(apiReader, ctx, res)
+	setupMockManager(manager, ctx, res)
+	setupMockDescriptor(descriptor, res)
+	setupMockUnstructuredConverter()
+
+	// Call
+	latest, err := r.Sync(ctx, sourceResource, *fieldExport)
+
+	//Assertions
+	require.Nil(err)
+	require.NotNil(latest.Status)
+	require.Len(latest.Status.Conditions, 0)
+	assertPatchedConfigMap(false, t, ctx, kc)
+	assertPatchedSecretWithKey(true, t, ctx, kc, "new-key")
+}
+
+func TestSync_SetKeyNameExplicitlyWithEmptyString(t *testing.T) {
+	// Setup
+	require := require.New(t)
+	// Mock resource creation
+	r, kc, apiReader := mockFieldExportReconciler()
+	descriptor, res, _ := mockDescriptorAndAWSResource()
+	manager := mockManager()
+	fieldExport := fieldExportWithKey(FieldExportNamespace, FieldExportName, ackv1alpha1.FieldExportOutputTypeSecret, "")
+	sourceResource, _, _ := mockSourceResource()
+	ctx := context.TODO()
+	statusWriter := &ctrlrtclientmock.StatusWriter{}
+
+	//Mock behavior setup
+	setupMockClientForFieldExport(kc, statusWriter, ctx, fieldExport)
+	setupMockApiReaderForFieldExport(apiReader, ctx, res)
+	setupMockManager(manager, ctx, res)
+	setupMockDescriptor(descriptor, res)
+	setupMockUnstructuredConverter()
+
+	// Call
+	latest, err := r.Sync(ctx, sourceResource, *fieldExport)
+
+	//Assertions
+	require.Nil(err)
+	require.NotNil(latest.Status)
+	require.Len(latest.Status.Conditions, 0)
+	assertPatchedConfigMap(false, t, ctx, kc)
+	assertPatchedSecret(true, t, ctx, kc)
+}
+
 // Assertions
 
 func assertPatchedConfigMap(expected bool, t *testing.T, ctx context.Context, kc *ctrlrtclientmock.Client) {
@@ -492,6 +582,24 @@ func assertPatchedSecret(expected bool, t *testing.T, ctx context.Context, kc *c
 	}
 }
 
+func assertPatchedSecretWithKey(expected bool, t *testing.T, ctx context.Context, kc *ctrlrtclientmock.Client, key string) {
+	dataMatcher := mock.MatchedBy(func(cm *corev1.Secret) bool {
+		if cm.Data == nil {
+			return false
+		}
+		val, ok := cm.Data[key]
+		if !ok {
+			return false
+		}
+		return bytes.Equal(val, []byte("test-book-name"))
+	})
+	if expected {
+		kc.AssertCalled(t, "Patch", ctx, dataMatcher, mock.Anything)
+	} else {
+		kc.AssertNotCalled(t, "Patch", ctx, dataMatcher, mock.Anything)
+	}
+}
+
 // assertRecoverableCondition asserts that 'ConditionTypeRecoverable' condition
 // is present in the resource's status and that it's value is equal to
 // 'conditionStatus' parameter