Bug: fail on missing secret

[lukeelten]

The data source rancher2_secret_v2 does not fail when the referenced secret is not found. Therefore terraform simply runs further and eventually will face an error when the secret is used.

The expected behavior is, that terraform stops running and exists with an error when the referenced secret is not found.

[richardcase]

Thanks @lukeelten . The change seems reasonable to me.

Is there an issue for this?

[lukeelten]

No, I don't think there is an issue for that yet.

[kkaempf]

@snasovich - can someone from your team review and merge ?

[snasovich]

@kkaempf @mbologna , apologies for missing the ping on this.

@lukeelten , our team doesn't specifically own TFP altogether or Secret resource specifically but I see that we generally return error in case there are no matching resources found. However, there are also examples where the same pattern of returning nil on "Not Found" errors in used, see

terraform-provider-rancher2/rancher2/data_source_rancher2_catalog_v2.go

Lines 83 to 89 in c0f5eca

	if err != nil {
	if IsNotFound(err) || IsForbidden(err) {
	log.Printf("[INFO] Catalog V2 %s not found at cluster %s", name, clusterID)
	d.SetId("")
	return nil
	}
	return err

So I'm a little hesitant to change the behavior here as there may be users that rely on it returning nil instead of error. Could you please submit issue with clear repro steps and link this PR to it so we can gauge regression potential and such.

@cbron @olblak @prachidamle , not sure which team owns Secrets management but heads-up to you on this PR.

[cbron]

@lukeelten is this specific to secrets and not other types ? I agree with @snasovich that an issue describing specifically what situation you are trying to solve would help. As he said, that pattern is fairly common in our codebase, and we should look at changing the wider pattern instead of an isolated instance: https://github.com/search?q=repo%3Arancher%2Fterraform-provider-rancher2%20if%20IsNotFound(err)&type=code
Wouldn't this also qualify as a breaking change ? If so it would have to ship in a minor release.
On who owns secrets, I'm not really sure as it's a core upstream type, we would have to look into it.

[kkaempf]

Moving from Highlander to Platform Team project board, so it doesn't get lost. Please move to final board once an owner has been identified.

[lukeelten]

I opened an issue with steps to reproduce. #1337

[lukeelten]

@snasovich
I agree with your hesitance. I did not notice that this behavior is also present in other resources. Nevertheless when I create a terraform script and add a secret as data source, I expect terraform to fail if the secret is not present. That is the whole purpose of a datasource.

I also agree with the assessment that this is a breaking change. But in my opinion you should only apply this pattern to datasources and not to all resources.

[matttrach]

I agree with the OP on this one, using Data resources to ensure that objects exist is an expected pattern when using Terraform.