WIP: Define Value interface

[errordeveloper]

This is to allow passing references and other intrinsics when constructing a template.

To keep things simple to start with, I've implemented this as a breaking change. We can settle on how it should be done once we have a clearer idea of the exact approach.

Replaces #95.

[errordeveloper]

So I guess there is one obvious thing here, we need more types and some kind of place-holder type.

Besides that, in eksctl I've encountered that I need this:

  NodeGroup:
    Type: AWS::AutoScaling::AutoScalingGroup
    Properties:
      DesiredCapacity: !Ref NodeAutoScalingGroupMaxSize
      LaunchConfigurationName: !Ref NodeLaunchConfig
      MinSize: !Ref NodeAutoScalingGroupMinSize
      MaxSize: !Ref NodeAutoScalingGroupMaxSize
      VPCZoneIdentifier:
        !Ref Subnets
      Tags:
      - Key: Name
        Value: !Sub "${ClusterName}-${NodeGroupName}-Node"
        PropagateAtLaunch: 'true'
      - Key: !Sub 'kubernetes.io/cluster/${ClusterName}'
        Value: 'owned'
        PropagateAtLaunch: 'true'
    UpdatePolicy:
      AutoScalingRollingUpdate:
        MinInstancesInService: '1'
        MaxBatchSize: '1'

First up, cloudformation.Tag doesn't have PropagateAtLaunch. Secondly, VPCZoneIdentifier is currently []string, and with this PR it becomes []*cloudformation.StringIntrinsic... What we need is something that would allow slice of strings, or an an intrinsic... Not sure what is the right answer at the moment. In my code I've simply defined all properties as a map.

[errordeveloper]

I have finished implementing all of what we need in eksctl-io/eksctl#132. It would be great to hear how we could go about changes akin to what's shown here.

[PaulMaddox]

I started messaging you back on Slack, but figured the feedback was probably better suited to discussion here.

1. All this change in a single commit is pretty hard to grok.

• Tests & Documentation for this are required

• Agree with your comments on Slack that using an interface will be nicer

• Can we rename StringIntrinsic to StringReference - intrinsic could mean anything from Fn::Join etc, where as we are specifically dealing with References here.

• Rename cloudformation.NewStringIntrinsic(string) to cloudformation.NewStringReference(name), and I also think we should create a cloudformation.Ref(name) convenience method (to be used in documentation) as 99% of use cases will just be for a string.

[errordeveloper]

@PaulMaddox I decided to rework this because my use-case already requires Fn::GetAtt, Fn::ImportValue, Fn::Join, Fn::Split and Fn::Sub. So I'll rework it to use an interface and make type naming more sensible. I'm happy to add docs and tests, but only after we agree on weather the direction is overall plausible.

[errordeveloper]

@PaulMaddox I've re-written this, please take a look at c0c0166.

[PaulMaddox]

This is a lot cleaner - I like the approach.

Do you have any thoughts about what the API would look like for setting Value values? Obviously we can't just use strings now :(

Something like the aws.String("") method in the aws sdk maybe? cloudformation.String("") ?

[errordeveloper]

Yes, it will be cloudformation.String(""). Here is an edited example from eksctl code:

func addResources(globalCIDR *net.IPNet, subnets map[string]*net.IPNet) {
	t.Resource["VPC"] = &cloudformation.AWSEC2VPC{
		CidrBlock:          cloudformation.String(globalCIDR.String()),
		EnableDnsSupport:   cloudformation.True,
		EnableDnsHostnames: cloudformation.True,
	}
        refVPC := cloudformation.MakeRef("VPC")

        t.Resource["InternetGateway"] = &cloudformation.AWSEC2InternetGateway{})
        refIG := cloudformation.MakeRef("InternetGateway")
	t.Resource["VPCGatewayAttachment"] = &cloudformation.AWSEC2VPCGatewayAttachment{
		InternetGatewayId: refIG,
		VpcId:             refVPC,
	}

	t.Resources["RouteTable"] = &cloudformation.AWSEC2RouteTable{
		VpcId: refVPC,
	}
        refRT := cloudformation.MakeRef("RouteTable")

	t.Resources["PublicSubnetRoute"] = &cloudformation.AWSEC2Route{
		RouteTableId:         refRT,
		DestinationCidrBlock: cloudformation.String("0.0.0.0/0"),
		GatewayId:            refIG,
	}

        subnetRefs := []cloudformation.Value{}
	for az, subnet := range subnets {
		alias := strings.ToUpper(strings.Join(strings.Split(az, "-"), ""))
		t.Resources["Subnet"+alias] = &cloudformation.AWSEC2Subnet{
			AvailabilityZone: cloudformation.String(az),
			CidrBlock:        cloudformation.String(subnet.String()),
			VpcId:            refVPC,
		}
                refSubnet := cloudformation.MakeRef("Subnet"+alias)
		t.Resources["RouteTableAssociation"+alias] = &cloudformation.AWSEC2SubnetRouteTableAssociation{
			SubnetId:     refSubnet,
			RouteTableId: refRT,
		}
		subnetRefs = append(subnetRefs, refSubnet)
	}

	t.Resources["ControlPlaneSecurityGroup"] = &cloudformation.AWSEC2SecurityGroup{
		GroupDescription: cloudformation.String("Communication between the control plane and worker node groups"),
		VpcId:            refVPC,
	})
       
	t.Resources["ControlPlane"] = & cloudformation.AWSEKSCluster{
		Name:    cloudformation.String(c.spec.ClusterName),
		RoleArn: cloudformation.MakeFnGetAtt("ServiceRole.Arn"),
		Version: cloudformation.String(version),
		ResourcesVpcConfig: & cloudformation.AWSEKSCluster_ResourcesVpcConfig{
			SubnetIds:        subnetRefs,
			SecurityGroupIds:  []cloudformation.Value{cloudformation.MakeRef("ControlPlaneSecurityGroup")},
		},
	}
}

However, in eksctl has some helpers, e.g.

func (r *resourceSet) newResource(name string, properties interface{}) cloudformation.Value

This function adds the resource with t.Resources[name] = properties and returns cloudformation.MakeRef(name) which you can capture whenever you need it.

so this

	t.Resource["VPC"] = &cloudformation.AWSEC2VPC{
		CidrBlock:          cloudformation.String(globalCIDR.String()),
		EnableDnsSupport:   cloudformation.True,
		EnableDnsHostnames: cloudformation.True,
	}
        refVPC := cloudformation.MakeRef("VPC")

becomes

	refVPC := r.newResource("VPC", &cloudformation.AWSEC2VPC{
		CidrBlock:          cloudformation.String(globalCIDR.String()),
		EnableDnsSupport:   cloudformation.True,
		EnableDnsHostnames: cloudformation.True,
	}

If you are intertested, I'm happy to add this here. We have other syntactic sugar, but is a bit more specific to eksctl, and I wouldn't hurry to suggest it here.

[errordeveloper]

I'll go ahead and try to fix the tests, then see what new tests can be added also.

[errordeveloper]

@PaulMaddox please review commits separately, not checking generated code doesn't show that tests pass, so I had to do that.

[PaulMaddox]

I have #114 in a good working state now, so am closing this PR in favour of that one