Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make fields of type TypeList in the schema generate an attribute diff when individual elements of that list change #1103

Open
aShevc opened this issue Nov 16, 2022 · 0 comments
Labels
enhancement New feature or request

Comments

@aShevc
Copy link

aShevc commented Nov 16, 2022

SDK version

v2.24.0

Use-cases

I am currenlty working on a contribution to https://github.com/hashicorp/terraform-provider-aws that should allow for updating certain objects in AWS Sagemaker service called Feature Groups. This is done via UpdateFeatureGroup request.

To accommodate this, the following needs to be implemented. In the repository we have a resource called Sagemaker Feature Group defined like this. It has the field of TypeList called feature_definition defined at this line. Due to the current Sagemaker service limitations we are able to add new items to the list, but we can not change existing items. Hence, we need to develop a solution that allows for the following:

  • Allow for the new items to be added to that list without forcing creation of a new ResourceFeatureGroup resource
  • Force creation of a new instance of ResourceFeatureGroup resource when the existing elements are deleted from the list, or when the existing items change in any way.

For the reference, the ResourceFeatureGroup schema looks the following way:

schema.Resource{
                ...
		Schema: map[string]*schema.Schema{
			"feature_definition": {
				Type:     schema.TypeList,
				Required: true,
				ForceNew: false,
				MinItems: 1,
				MaxItems: 2500,
				Elem: &schema.Resource{
					Schema: map[string]*schema.Schema{
						"feature_name": {
							Type:     schema.TypeString,
							Optional: true,
							ValidateFunc: validation.All(
								validation.StringLenBetween(1, 64),
								validation.StringNotInSlice([]string{"is_deleted", "write_time", "api_invocation_time"}, false),
								validation.StringMatch(regexp.MustCompile(`^[a-zA-Z0-9]([-_]*[a-zA-Z0-9]){0,63}`),
									"Must start and end with an alphanumeric character and Can only contains alphanumeric characters, hyphens, underscores. Spaces are not allowed."),
							),
						},
						"feature_type": {
							Type:         schema.TypeString,
							Optional:     true,
							ValidateFunc: validation.StringInSlice(sagemaker.FeatureType_Values(), false),
						},
					},
				},
			},

So currently I was able to keep forcing the resource replacement when items deleted from the list, while simultaneously allowing the change of the resource when new items added. This is done via implementing a CustomizeDiff function as so:

CustomizeDiff: customdiff.All(
			customdiff.ForceNewIf("feature_definition", func(_ context.Context, d *schema.ResourceDiff, meta interface{}) bool {
				old, new := d.GetChange("feature_definition")
				var featureDefinitionsOld = expandFeatureGroupFeatureDefinition(old.([]interface{}))
				var featureDefinitionsNew = expandFeatureGroupFeatureDefinition(new.([]interface{}))

				return len(featureDefinitionsNew) < len(featureDefinitionsOld)
			}),

From here, I also need to be able to force the creation of a ResourceFeatureGroup resource when individual elements in a feature_definition list change, but the length of the list stays the same. I was not able to do so, unfortunately.

The reason for it is the following. Even if I return true from the same CustomizeDiff function, as below, the result of the function is ignored.


CustomizeDiff: customdiff.All(
			customdiff.ForceNewIf("feature_definition", func(_ context.Context, d *schema.ResourceDiff, meta interface{}) bool {
    old, new := d.GetChange("feature_definition")
    var featureDefinitionsOld = expandFeatureGroupFeatureDefinition(old.([]interface{}))
    var featureDefinitionsNew = expandFeatureGroupFeatureDefinition(new.([]interface{}))

    if (len(featureDefinitionsNew) < len(featureDefinitionsOld)) {
        return true
    } else {
        return forceNewIfNoFeatureDefinitionFound(featureDefinitionsOld, featureDefinitionsNew)
    }
}),

func forceNewIfNoFeatureDefinitionFound(featureDefinitionsOld []*sagemaker.FeatureDefinition,
                                                                       featureDefinitionsNew []*sagemaker.FeatureDefinition) bool {

    var res = false

    for _, elem := range featureDefinitionsOld {
	var elementExists = false
	for _, newElem := range featureDefinitionsNew {
		if *newElem.FeatureName == *elem.FeatureName {
			elementExists = true
		break
	    }
        }

        if !elementExists {
            res = true
        }
    }

    return res
 }

So from what I am seeing right here, the change of the individual element in a TypeList actually does not generate an attribute diff in that field, hence the result of the ForceNewIf function is ignored as it is said in the doc on this function here.

The same happens when I implement CustomizeDiff as a custom CustomizeDiffFunc function in which I try to force resource creation by calling ForceNew on the Resource instance as such:

CustomizeDiff:  func(_ context.Context, d *schema.ResourceDiff, meta interface{}) error {
    old, new := d.GetChange("feature_definition")

    var featureDefinitionsOld = expandFeatureGroupFeatureDefinition(old.([]interface{}))
    var featureDefinitionsNew = expandFeatureGroupFeatureDefinition(new.([]interface{}))

    if (forceNewIfNoFeatureDefinitionFound(featureDefinitionsOld, featureDefinitionsNew)) {
        return d.ForceNew("feature_definition")
    }

   return nil
},

Attempted Solutions

There is another potential solution I have added, and this is implementing a CustomizeDiff on the individual elements in a feature_definition list as such:

Elem: &schema.Resource{
 CustomizeDiff:  func(_ context.Context, d *schema.ResourceDiff, meta interface{}) error {
    old, new := d.GetChange("feature_name")

    var featureNameOld = old.(string)
    var featureNameNew = new.(string)

    if featureNameOld != "" && featureNameNew != featureNameNew {
        d.ForceNew("feature_name")
    }
   
    return nil
},

This however also does not have any effect. In fact, it seems that the change in featureName seems not to trigger CustomizeDiff function at all. Since even calling panic inside this function does not force any effect.

In the end, I ended up thinking that the only option I have is to combine the first variant of a CustomizeDiff with the functionality that will throw an error when individual elements of a feature_definition list change upon calling an update function. This feels ugly and I am really hesitant to make it a final solution.

Proposal

I see the two potential resolutions to the problem:

  • Upon changes to the individual elements of the fields of TypeList, these changes should also generate a diff in the field itself. Or this case should be treated differently than other cases, so that in case if those changes are made the result of methods ForceNewIfChange and the result of the method ForceNew of the Resource type are not ignored.
  • The second, less obvious and less desirable way would be to treat the CustomizeDiff that is defined at the element level in a TypeList field differently. Namely, when elements of the list are defined as Schema objects, and individual elements of that elements change, these should trigger the CustomizeDiff function defined at the element level.

I am also open to suggestions on any alternatives, that would allow me to achieve my aforementioned goal of simultaneously force creation of new resource when the existing elements of the TypeList change.

Also, let me know if the issue described here looks more like a bug.

References

For reference on the original issue in terraform-provider-aws plese see: hashicorp/terraform-provider-aws#26809

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant