Skip to content

EnvironmentConfig with empty data field cause errors #80

@markussiebert

Description

@markussiebert

We should modify the mergeEnvConfigsData function to explicitly skip EnvironmentConfigs that have a nil data field. According to the EnvironmentConfig definition, the data field is optional:

type EnvironmentConfig struct {
    metav1.TypeMeta   `json:",inline"`
    metav1.ObjectMeta `json:"metadata,omitempty"`

    // The data of this EnvironmentConfig.
    // This may contain any kind of structure that can be serialized into JSON.
    // +optional
    Data map[string]extv1.JSON `json:"data,omitempty"`
}

https://pkg.go.dev/github.com/crossplane/crossplane/apis/apiextensions/v1alpha1#EnvironmentConfig

Currently, when an EnvironmentConfig with a nil data field is processed, the function attempts to get the data field value which will lead to an (unexpected) error. We should explicitly skip these configs to ensure proper handling of empty EnvironmentConfigs.

Proposed Change

Add a check to skip EnvironmentConfigs with nil data fields

if err := fieldpath.Pave(c.Object).GetValueInto("data", &data); err != nil {

func mergeEnvConfigsData(configs []unstructured.Unstructured) (map[string]interface{}, error) {
	merged := map[string]interface{}{}
	for _, c := range configs {
// changes start
		if c.Object["data"] == nil {
			continue // skip if no data field
		} 
// changes end
		data := map[string]interface{}{}
		if err := fieldpath.Pave(c.Object).GetValueInto("data", &data); err != nil {
			return nil, errors.Wrapf(err, "cannot get data from environment config %q", c.GetName())
		}

		merged = mergeMaps(merged, data)
	}
	return merged, nil
}

Proposed Test

...
                "MergeNormalAndEmptyEnvironmentConfigs": {
			reason: "The Function should merge a normal EnvironmentConfig with an empty one",
			args: args{
				req: &fnv1.RunFunctionRequest{
					Meta: &fnv1.RequestMeta{Tag: "hello"},
					Input: resource.MustStructJSON(`{
						"apiVersion": "template.fn.crossplane.io/v1beta1",
						"kind": "Input",
						"spec": {
							"environmentConfigs": [
								{
									"type": "Reference",
									"ref": {
										"name": "normal-env-config"
									}
								},
								{
									"type": "Reference",
									"ref": {
										"name": "empty-env-config"
									}
								}
							]
						}
					}`),
					ExtraResources: map[string]*fnv1.Resources{
						"environment-config-0": {
							Items: []*fnv1.Resource{
								{
									Resource: resource.MustStructJSON(`{
									"apiVersion": "apiextensions.crossplane.io/v1beta1",
									"kind": "EnvironmentConfig",
									"metadata": {
										"name": "normal-env-config"
									},
									"data": {
										"key1": "value1",
										"key2": "value2"
									}
								}`),
								},
							},
						},
						"environment-config-1": {
							Items: []*fnv1.Resource{
								{
									Resource: resource.MustStructJSON(`{
									"apiVersion": "apiextensions.crossplane.io/v1beta1",
									"kind": "EnvironmentConfig",
									"metadata": {
										"name": "empty-env-config"
									}
								}`),
								},
							},
						},
					},
				},
			},
			want: want{
				rsp: &fnv1.RunFunctionResponse{
					Meta:    &fnv1.ResponseMeta{Tag: "hello", Ttl: durationpb.New(response.DefaultTTL)},
					Results: []*fnv1.Result{},
					Requirements: &fnv1.Requirements{
						ExtraResources: map[string]*fnv1.ResourceSelector{
							"environment-config-0": {
								ApiVersion: "apiextensions.crossplane.io/v1beta1",
								Kind:       "EnvironmentConfig",
								Match: &fnv1.ResourceSelector_MatchName{
									MatchName: "normal-env-config",
								},
							},
							"environment-config-1": {
								ApiVersion: "apiextensions.crossplane.io/v1beta1",
								Kind:       "EnvironmentConfig",
								Match: &fnv1.ResourceSelector_MatchName{
									MatchName: "empty-env-config",
								},
							},
						},
					},
					Context: &structpb.Struct{
						Fields: map[string]*structpb.Value{
							FunctionContextKeyEnvironment: structpb.NewStructValue(resource.MustStructJSON(`{
								"apiVersion": "internal.crossplane.io/v1alpha1",
								"kind": "Environment",
								"key1": "value1",
								"key2": "value2"
							}`)),
						},
					},
				},
			},
		},
...

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions