pulumi / pulumi-eks

A Pulumi component for easily creating and managing an Amazon EKS Cluster
https://www.pulumi.com/registry/packages/eks/
Apache License 2.0
171 stars 82 forks source link

Unable to set NewNodeGroupV2's ExtraNodeSecurityGroups or NodeSecurityGroup using a SecurityGroupOutput #829

Closed MMartyn closed 1 year ago

MMartyn commented 1 year ago

What happened?

This was working fine up until v1 but now when I try to set the NodeSecurityGroup or ExtraNodeSecurityGroups using the Cluster.NodeSecurityGroup I get a type error.

Steps to reproduce

Create a cluster and then try to create a v2 nodegroup using the cluster's nodesecuritygroup.

Expected Behavior

That I would be able to pass that into v2 node group and it would work.

Actual Behavior

It doesn't/

Output of pulumi about

No response

Additional context

Using the pulumi-eks in go

Contributing

Vote on this issue by adding a 👍 reaction. To contribute a fix for this issue, leave a comment (and link to your pull request, if you've opened one already).

MMartyn commented 1 year ago

Pretty sure it is a result of this change

lblackstone commented 1 year ago

@justinvp It looks like this may be related to your change, so could you take a look?

justinvp commented 1 year ago

@MMartyn, can you share the snippet of code that worked previously and doesn't work anymore?

With 1.0, you should be able to pass-in the cluster to NewNodeGroupV2 directly, e.g.:

cluster, err := eks.NewCluster(ctx, "mycluster", &eks.ClusterArgs{
    SkipDefaultNodeGroup: pulumi.BoolPtr(true),
    InstanceRole:         role0,
})
if err != nil {
    return err
}

if _, err := eks.NewNodeGroupV2(ctx, "example-ng2", &eks.NodeGroupV2Args{
    Cluster:         cluster,
    InstanceType:    pulumi.StringPtr("t2.medium"),
    DesiredCapacity: pulumi.IntPtr(1),
    MinSize:         pulumi.IntPtr(1),
    MaxSize:         pulumi.IntPtr(2),
    Labels:          pulumi.StringMap{"ondemand": pulumi.String("true")},
    InstanceProfile: instanceProfileO,
}); err != nil {
    return err
}
MMartyn commented 1 year ago

Think you may have mixed up the 2 issues I opened today. This one is passing a SecurityGroupOutput into a NewNodeGroupV2 in either the NodeSecurityGroup or ExtraNodeSecurityGroup.

MMartyn commented 1 year ago

Little snippet that will show the issue. In Go, create an eks.Cluster and then try creating a NodeGroupV2 with the following:

ExtraNodeSecurityGroups: ec2.SecurityGroupArray{
  cluster.NodeSecurityGroup,
},
justinvp commented 1 year ago

I see, thanks for the extra information, @MMartyn.

Let me take a look at #831. I think it may have previously accidentally worked when the schema had these as inputty, but we should also look at updating the TypeScript implementation to accept Input<T> to ensure things really work as intended.

vboulineau commented 1 year ago

@justinvp @lblackstone We're encountering the same issue with ExtraNodeSecurityGroup. Any of you can have a look at the proposed PR?