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

Generated code does not compile if all fields are ignored #1069

Open
muvaf opened this issue Nov 17, 2021 · 6 comments
Open

Generated code does not compile if all fields are ignored #1069

muvaf opened this issue Nov 17, 2021 · 6 comments
Assignees
Labels
area/code-generation Issues or PRs as related to controllers or docs code generation kind/bug Categorizes issue or PR as related to a bug. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness.

Comments

@muvaf
Copy link
Contributor

muvaf commented Nov 17, 2021

Describe the bug

There are some resources with very few fields, like KMS Alias, and we may end up adding all of their fields to the ignore list. However, since there are for loops running for resources like KMS Alias where the read function returns a list, the for loop ends up having empty content and since elem is not used, the code does not compile.

An example for loop generated is as following:

func GenerateAlias(resp *svcsdk.ListAliasesOutput) *svcapitypes.Alias {
	cr := &svcapitypes.Alias{}

	found := false
	for _, elem := range resp.Aliases {
		found = true
		break
	}
	if !found {
		return cr
	}

	return cr
}

Compiler does not accept having elem there as unused.

Steps to reproduce

Ignore all fields in KMS Alias resource and run the generator.

Expected outcome

I'd expected the code to compile.

Environment

KMS Alias.
Code Generator Commit: cac5654b7bb64c8f754ad9af01799ef70d9541b6

@muvaf muvaf added the kind/bug Categorizes issue or PR as related to a bug. label Nov 17, 2021
@jaypipes jaypipes added the area/code-generation Issues or PRs as related to controllers or docs code generation label Nov 17, 2021
@jaypipes jaypipes self-assigned this Nov 17, 2021
@jaypipes
Copy link
Collaborator

@muvaf Hi! So, in the case you highlighted here, if we did an throwaway assignment of elem to _ directly after the for loop, the code would compile but if there are multiple Aliases returned from the call to ListAliases, the cr returned would be an empty svcapitypes.Alias struct. Is that what you would want? Seems like buggy behaviour to me.

@jaypipes jaypipes removed their assignment Nov 17, 2021
@muvaf
Copy link
Contributor Author

muvaf commented Nov 17, 2021

the code would compile but if there are multiple Aliases returned from the call to ListAliases, the cr returned would be an empty svcapitypes.Alias struct. Is that what you would want?

Yes, and that's because there is no field remained on the resource to set anyway. So, it's already bound to return an empty struct.

In our case, those fields are ignored because we needed special handling of them, hence moved them to CustomAliasParameters that is inlined in spec, so, we process that empty svcapitypes.Alias in another function called right after this one.

@ezgidemirel
Copy link

Hi @jaypipes, I encountered the same issue with Neptune DB Instance resource. Is there a solution/workaround you may suggest?

if resp.DBInstance.DBSecurityGroups != nil {
		f14 := []*string{}
		for _, f14iter := range resp.DBInstance.DBSecurityGroups {
			var f14elem string
			f14 = append(f14, &f14elem)
		}
		cr.Spec.ForProvider.DBSecurityGroups = f14
	} else {
		cr.Spec.ForProvider.DBSecurityGroups = nil
	}

@jaypipes
Copy link
Collaborator

Hi @jaypipes, I encountered the same issue with Neptune DB Instance resource. Is there a solution/workaround you may suggest?

if resp.DBInstance.DBSecurityGroups != nil {
		f14 := []*string{}
		for _, f14iter := range resp.DBInstance.DBSecurityGroups {
			var f14elem string
			f14 = append(f14, &f14elem)
		}
		cr.Spec.ForProvider.DBSecurityGroups = f14
	} else {
		cr.Spec.ForProvider.DBSecurityGroups = nil
	}

@ezgidemirel apologies for the super-late response on this :(

The above is likely not the same issue. If I were to guess, the Neptune API shares a lineage with the RDS API and the shape of the DBSecurityGroups field in the Create Input and Output shapes is a different type, which is what is leading to the above problem.

We solved this in the RDS controller using this block of YAML in the generator.yaml file

@ezgidemirel
Copy link

Thanks @jaypipes! I'll give it a try.

@ack-bot ack-bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label May 17, 2022
@ack-bot ack-bot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Jun 16, 2022
@ack-bot ack-bot closed this as completed Jul 16, 2022
@haarchri
Copy link

/reopen

@aws-controllers-k8s aws-controllers-k8s deleted a comment from ack-bot Jul 18, 2022
@aws-controllers-k8s aws-controllers-k8s deleted a comment from haarchri Jul 18, 2022
@aws-controllers-k8s aws-controllers-k8s deleted a comment from ack-bot Jul 18, 2022
@aws-controllers-k8s aws-controllers-k8s deleted a comment from ack-bot Jul 18, 2022
@aws-controllers-k8s aws-controllers-k8s deleted a comment from ack-bot Jul 18, 2022
@aws-controllers-k8s aws-controllers-k8s deleted a comment from ack-bot Jul 18, 2022
@jaypipes jaypipes added lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. and removed lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. labels Jul 18, 2022
@jaypipes jaypipes reopened this Jul 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/code-generation Issues or PRs as related to controllers or docs code generation kind/bug Categorizes issue or PR as related to a bug. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness.
Projects
None yet
Development

No branches or pull requests

5 participants