aws-controllers-k8s / community

AWS Controllers for Kubernetes (ACK) is a project enabling you to manage AWS services from Kubernetes
https://aws-controllers-k8s.github.io/community/
Apache License 2.0
2.39k stars 253 forks source link

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

Open muvaf opened 2 years ago

muvaf commented 2 years ago

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

jaypipes commented 2 years ago

@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.

muvaf commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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

haarchri commented 2 years ago

/reopen