aws: Add check for multiple worker pools

[knrt10]

Add validation check for multiple worker pools, to avoid conflicting
port values in aws_lb_listener. User has to set unique values for
lb_http_port and lb_https_port.

Part of #839

Signed-off-by: knrt10 kautilya@kinvolk.io

[invidian]

Also @knrt10 perhaps we could implement some unit tests to validate this logic? It shouldn't be too hard, as we do similar thing for example for Packet already.

[invidian]

I'd suggest the following patch to resolve the issues I found. Also, those scenarios should be covered by tests as well.

diff --git a/pkg/platform/aws/aws.go b/pkg/platform/aws/aws.go
index f1dd9f87..eea2a5e6 100644
--- a/pkg/platform/aws/aws.go
+++ b/pkg/platform/aws/aws.go
@@ -294,18 +294,40 @@ func (c *config) checkValidConfig() hcl.Diagnostics {
 func (c *config) checkDifferentPortForMuliplePools() hcl.Diagnostics {
        var diagnostics hcl.Diagnostics

-       portMap := make(map[int]bool)
+       portMap := make(map[int]struct {
+               field    string
+               poolName string
+       })

        for _, wp := range c.WorkerPools {
-               if !portMap[wp.LBHTTPPort] && !portMap[wp.LBHTTPSPort] {
-                       portMap[wp.LBHTTPPort] = true
-                       portMap[wp.LBHTTPSPort] = true
-               } else {
-                       diagnostics = append(diagnostics, &hcl.Diagnostic{
-                               Severity: hcl.DiagError,
-                               Summary:  "Unique values required",
-                               Detail:   fmt.Sprintf("Values lb_http_port and lb_https_port not set for worker pool %s", wp.Name),
-                       })
+               httpPort := wp.LBHTTPPort
+               if httpPort == 0 {
+                       httpPort = 80
+               }
+
+               httpsPort := wp.LBHTTPSPort
+               if httpsPort == 0 {
+                       httpsPort = 443
+               }
+
+               ports := map[int]string{
+                       httpPort:  "lb_http_port",
+                       httpsPort: "lb_https_port",
+               }
+
+               for p, field := range ports {
+                       if v, ok := portMap[p]; ok {
+                               diagnostics = append(diagnostics, &hcl.Diagnostic{
+                                       Severity: hcl.DiagError,
+                                       Summary:  "Unique ports required",
+                                       Detail:   fmt.Sprintf("'worker_pool.%s.%s' collides with 'worker_pool.%s.%s'", wp.Name, field, v.poolName, v.field),
+                               })
+                       }
+
+                       portMap[p] = struct {
+                               field    string
+                               poolName string
+                       }{field, wp.Name}
                }
        }

[knrt10]

Updated

[invidian]

Just some nits, but not blocking. Looks cool @knrt10!

[iaguis]

Small clarification.

[iaguis]

LGTM