added review changes

modules/lambda-registrator/main.tf

@@ -160,29 +160,29 @@ resource "random_id" "repo_id" {
 }
 
 resource "aws_ecr_repository" "lambda-registrator" {
-  count        = var.ecr_image_uri != "" ? 0 : 1
+  count        = var.enable_auto_publish_ecr_image ? 1 : 0
   name         = local.ecr_repo_name
   force_delete = true
 }
 
 resource "docker_image" "lambda_registrator" {
-  count = var.ecr_image_uri != "" ? 0 : 1
+  count = var.enable_auto_publish_ecr_image ? 1 : 0
   name  = var.consul_lambda_registrator_image
 }
 
 resource "docker_tag" "lambda_registrator_tag" {
-  count        = var.ecr_image_uri != "" ? 0 : 1
+  count        = var.enable_auto_publish_ecr_image ? 1 : 0
   source_image = docker_image.lambda_registrator[count.index].name
   target_image = local.generated_ecr_image_uri
 }
 resource "docker_registry_image" "push_image" {
-  count         = var.ecr_image_uri != "" ? 0 : 1
+  count         = var.enable_auto_publish_ecr_image ? 1 : 0
   name          = docker_tag.lambda_registrator_tag[count.index].target_image
   keep_remotely = true
 }
 
 resource "aws_lambda_function" "registration" {
-  image_uri                      = var.ecr_image_uri != "" ? var.ecr_image_uri : local.generated_ecr_image_uri
+  image_uri                      = var.enable_auto_publish_ecr_image ? local.generated_ecr_image_uri : var.ecr_image_uri
   package_type                   = "Image"
   function_name                  = var.name
   role                           = aws_iam_role.registration.arn

modules/lambda-registrator/validations.tf

@@ -0,0 +1,3 @@
+locals {
+  require_ecr_image_uri_or_enable_auto_publish_ecr_image_set = var.ecr_image_uri == "" && var.enable_auto_publish_ecr_image == false ? file("ERROR: either ecr_image_uri or enable_auto_publish_ecr_image must be set") : null
+}

modules/lambda-registrator/variables.tf

@@ -84,6 +84,7 @@ variable "ecr_image_uri" {
   repository or configuring pull through cache rules (https://docs.aws.amazon.com/AmazonECR/latest/userguide/pull-through-cache.html).
   EOT
   type        = string
+  default     = ""
 }
 
 variable "sync_frequency_in_minutes" {
@@ -131,4 +132,10 @@ variable "docker_host" {
   description = "The docker socket for your system"
   type        = string
   default     = "unix:///var/run/docker.sock"
+}
+
+variable "enable_auto_publish_ecr_image" {
+  description = "enables auto pushing public image to private ecr repo if set to true"
+  type        = bool
+  default     = false
 }

test/acceptance/setup-terraform/ecr.tf

@@ -8,6 +8,7 @@ locals {
 }
 
 resource "aws_ecr_repository" "lambda-registrator" {
+  count        = var.enable_auto_publish_ecr_image ? 1 : 0
   name         = local.ecr_repository_name
   force_delete = true
 }

test/acceptance/setup-terraform/variables.tf

@@ -5,3 +5,9 @@ variable "region" {
   default     = "us-west-2"
   description = "AWS region"
 }
+
+variable "enable_auto_publish_ecr_image" {
+  description = "enables auto pushing public image to private ecr repo if set to true"
+  type        = bool
+  default     = false
+}

test/acceptance/tests/basic_test.go

@@ -10,6 +10,7 @@ import (
 	"net/http"
 	"os"
 	"os/exec"
+	"regexp"
 	"strings"
 	"testing"
 	"time"
@@ -31,9 +32,11 @@ type SetupConfig struct {
 
 func TestBasic(t *testing.T) {
 	cases := map[string]struct {
-		secure                 bool
-		enterprise             bool
-		autoPublishRegistrator bool
+		secure                            bool
+		enterprise                        bool
+		autoPublishRegistrator            bool
+		ecrImageNotSet                    bool
+		ecrImageUriOrAutoPublishNotSetErr string
 	}{
 		"secure": {
 			secure: true,
@@ -49,6 +52,11 @@ func TestBasic(t *testing.T) {
 			secure:                 true,
 			autoPublishRegistrator: true,
 		},
+		"secure ecrImageUri not set": {
+			secure:                            true,
+			ecrImageNotSet:                    true,
+			ecrImageUriOrAutoPublishNotSetErr: "ERROR: either ecr_image_uri or enable_auto_publish_ecr_image must be set",
+		},
 	}
 
 	for name, c := range cases {
@@ -60,9 +68,6 @@ func TestBasic(t *testing.T) {
 			partition := ""
 			queryString := ""
 			tfVars["consul_image"] = "public.ecr.aws/hashicorp/consul:1.15.1"
-			if c.autoPublishRegistrator {
-				tfVars["ecr_image_uri"] = ""
-			}
 			if c.enterprise {
 				tfVars["consul_license"] = os.Getenv("CONSUL_LICENSE")
 				require.NotEmpty(t, tfVars["consul_license"], "CONSUL_LICENSE environment variable is required for enterprise tests")
@@ -79,6 +84,24 @@ func TestBasic(t *testing.T) {
 			tfVars["suffix"] = suffix
 			tfVars["setup_suffix"] = setupSuffix
 
+			var setupCfg SetupConfig
+			if !c.autoPublishRegistrator && c.ecrImageNotSet {
+				delete(tfVars, "ecr_image_uri")
+				setupTerraformOptions := terraform.WithDefaultRetryableErrors(t, &terraform.Options{
+					TerraformDir: "./setup",
+					Vars:         tfVars,
+					NoColor:      true,
+				})
+				_, err := terraform.PlanE(t, setupTerraformOptions)
+				require.Error(t, err)
+				// error messages are wrapped, so a space may turn into a newline.
+				regex := strings.ReplaceAll(regexp.QuoteMeta(c.ecrImageUriOrAutoPublishNotSetErr), " ", "\\s+")
+				require.Regexp(t, regex, err.Error())
+				return
+			}
+			if c.autoPublishRegistrator {
+				tfVars["enable_auto_publish_ecr_image"] = true
+			}
 			setupTerraformOptions := terraform.WithDefaultRetryableErrors(t, &terraform.Options{
 				TerraformDir: "./setup",
 				Vars:         tfVars,
@@ -95,7 +118,6 @@ func TestBasic(t *testing.T) {
 
 			terraform.InitAndApply(t, setupTerraformOptions)
 
-			var setupCfg SetupConfig
 			require.NoError(t, UnmarshalTF("./setup", &setupCfg))
 
 			clientServiceName := fmt.Sprintf("test_client_%s", suffix)