Fix issue with with Opsworks and empty Custom Cook Book sources

[u2mejc]

This resolves #6077

[u2mejc]

I've confirmed this persists with HEAD: Terraform v0.6.15-dev (aef5bcf98227b0ac964657a29ef2a1ebeed19a51)

I've created a simple gist to demonstrate the fix against: https://gist.github.com/u2mejc/c1613ed663daa9bd3eb8749e8a658cfd

[catsby]

Replied to #6077, I'd like for us to considering not making this computed, but instead only saving the information when there is actual a custom cookbook returned

[u2mejc]

@catsby said:

Hey @u2mejc – do you have a simple config that demonstrates this problem? The computed flag tells Terraform that the API may return data and to not worry about it; the resource itself is not populating empty data on it's own. Or, it shouldn't 😄

I did some debugging, and found that the CustomCookbooksSource source that's returned from DescribeStacks call here is coming back empty, instead of the typical nil. As a result we're storing empty values when we should omit it if nothing there is found. Do you think you could adjust #6078 to account for that?

Moving development discussion here so we don't get too split brained. I'm guessing you saw the example to reproduce the issue above. ☝️

Replied to #6077, I'd like for us to considering not making this computed, but instead only saving the information when there is actual a custom cookbook returned

I'm not sure I understand, by not making this computed did you mean something different then what is already dropped in this PR? I thought Terraform was populating the empty strings intentioanlly when computed based on inline comments like this:

// If the key does exist in the schema but doesn't exist in the configuration,
// then the default value for that type will be returned. For strings, this is
// "", for numbers it is 0, etc.

Is that not what's expected?

[u2mejc]

@catsby I'm happy to be able to help, getting Terraform to fully support OpsWorks is aligned with my current sprint. But I'm not super familiar with the code base. Can you please give a little more context around your ask, I'll be happy to work on the code for you then. 👍

[u2mejc]

@apparentlymart I was wondering if you could look at this PR? I know you're probably most intimate with it.

Use case is for creating OpsWorks stacks without custom repos, namely for recipes that are all available in the supermarket. I think this change is pretty straightforward, can you think of a use case this wouldn't work?

[apparentlymart]

@u2mejc I'm not sure exactly what @catsby was getting at here, but I think what he's suggesting is that instead of changing the schema we would check for and ignore an empty cookbooks source.

So maybe in resourceAwsOpsworksSetStackCustomCookbooksSource the condition would become:

    if v != nil && *.v.Type != "" {
        // ...

This would mean that if the Opsworks API returns a the cookbooks source as a bunch of empty strings then we just keep the values we already had.

Hopefully @catsby can confirm whether that was what he was driving at.

I'm super rusty on this code so I don't have any better theories than his, but I would agree that setting it as Computed is probably just masking the issue by ignoring the diff, and so other related problems may still arise as a result.

[u2mejc]

Thank you Martin!

I don't believe that AWS should ever respond with computed values for custom cookbooks? But if @catsby believes it would be better to ignore Terraform creating empty strings, I'm happy to adjust the PR.

Clint, let me know if you have time to look at this and comment or if you'd like Paul / Martin to approve it.

[catsby]

Hi friends! I'm terribly sorry to have left you hanging here. I popped in, said a thing, and then left, leaving you not unable to understand my glaring typo 😄

I'd like for us to considering not making this computed, but instead only saving the information when there is actual a custom cookbook returned

What I meant was not changing the computed nature, but instead only saving data if it's non-empty. I alluded to that in my comment #6077 (comment) but I failed to speak clearly, sorry!

What I'm seeing here is that if you do not specify a custom cookbook, that the stack.CustomCookbooksSource still has an "empty" cookbook, which is getting saved to state.

So, what @apparentlymart said, in that we should verify it's not nil and also contains something meaningful. I would want to triple check that this is true though (the empty value)

I hope that clears things up! I apologize again for the science* silence

*edit: I don't think I ever apologize for science ;)

[u2mejc]

Thank you Clint!!! I'll rework the PR and shoot it back up.

[u2mejc]

@catsby Per your request, I've adjusted this PR to simply check for nil and empty string, instead of disabling the computed value. Please let me know if there is anything else you'd like me to add. 👍

[catsby]

Excellent, thank you!

[ghost]

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.