From fd24d0856340af36cbaca899a70988dcf40fec54 Mon Sep 17 00:00:00 2001
From: Eric Beard <ericzbeard@gmail.com>
Date: Fri, 1 Nov 2024 16:41:37 -0700
Subject: [PATCH 01/15] Modules folder

---
 .github/workflows/modules.sh                  |  8 ++
 .github/workflows/modules.yml                 | 35 +++++++
 .gitignore                                    |  2 +-
 docs/README.tmpl                              | 96 ++++++++++++++++---
 {test/modules => modules}/api-resource.yaml   |  0
 {test/modules => modules}/bucket-policy.yaml  | 11 ---
 {test/modules => modules}/cognito.yaml        |  0
 .../compliant-bucket.yaml                     | 41 +++++++-
 modules/encrypted-bucket.yaml                 | 17 ++++
 {test/modules => modules}/load-balancer.yaml  |  0
 {test/modules => modules}/rest-api.yaml       |  0
 .../vpc.yaml => modules/simple-vpc.yaml       |  2 +
 {test/modules => modules}/static-site.yaml    |  4 +-
 scripts/integ.sh                              |  3 +
 test/modules/ext-ref.yaml                     | 28 ------
 test/modules/https.yaml                       |  6 --
 test/webapp/webapp.yaml                       | 18 ++--
 17 files changed, 199 insertions(+), 72 deletions(-)
 create mode 100755 .github/workflows/modules.sh
 create mode 100644 .github/workflows/modules.yml
 rename {test/modules => modules}/api-resource.yaml (100%)
 rename {test/modules => modules}/bucket-policy.yaml (56%)
 rename {test/modules => modules}/cognito.yaml (100%)
 rename test/modules/bucket.yaml => modules/compliant-bucket.yaml (80%)
 create mode 100644 modules/encrypted-bucket.yaml
 rename {test/modules => modules}/load-balancer.yaml (100%)
 rename {test/modules => modules}/rest-api.yaml (100%)
 rename test/modules/vpc.yaml => modules/simple-vpc.yaml (98%)
 rename {test/modules => modules}/static-site.yaml (97%)
 delete mode 100644 test/modules/ext-ref.yaml
 delete mode 100644 test/modules/https.yaml

diff --git a/.github/workflows/modules.sh b/.github/workflows/modules.sh
new file mode 100755
index 00000000..774ae1f1
--- /dev/null
+++ b/.github/workflows/modules.sh
@@ -0,0 +1,8 @@
+#!/bin/bash
+
+set -eoux pipefail
+
+# Zip up the modules directory and create a sha256 hash
+mkdir -p dist
+zip -r dist/modules.zip modules
+sha256sum -b dist/modules.zip > dist/modules.sha256
diff --git a/.github/workflows/modules.yml b/.github/workflows/modules.yml
new file mode 100644
index 00000000..e084f4cc
--- /dev/null
+++ b/.github/workflows/modules.yml
@@ -0,0 +1,35 @@
+on:
+  push:
+    tags:
+    - 'm*'
+
+name: Create a module release from tag
+
+jobs:
+
+  release:
+    name: Release
+    needs: build
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout code
+        uses: actions/checkout@v4
+        with:
+          fetch-depth: 0
+
+      - uses: actions/download-artifact@v4.1.7
+        with:
+          name: dist
+          path: dist
+
+      - run: |
+          set -x
+          (echo "${GITHUB_REF##*/}"; echo; git cherry -v "$(git describe --abbrev=0 HEAD^)" | cut -d" " -f3-) > CHANGELOG
+          assets=()
+          for zip in ./dist/*.zip; do
+            assets+=("$zip")
+          done
+          gh release upload "${GITHUB_REF##*/}" "${assets[@]}"
+        env:
+          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+
diff --git a/.gitignore b/.gitignore
index 06ba2ad8..77ba0d60 100644
--- a/.gitignore
+++ b/.gitignore
@@ -34,4 +34,4 @@ local/
 
 .DS_Store
 plugin.so
-
+dist/
diff --git a/docs/README.tmpl b/docs/README.tmpl
index 22401d47..2ed575f2 100644
--- a/docs/README.tmpl
+++ b/docs/README.tmpl
@@ -280,14 +280,11 @@ See `test/webapp/README.md` for a complete example of using these commands with
 #### Module
 
 The `!Rain::Module` directive is an experimental feature that allows you to
-create local modules of reuseable code that can be inserted into templates. A
-rain module is similar in some ways to a CDK construct, in that a module can
-extend existing resources, allowing the user of the module to override
-properties. For example, your module could extend an S3 bucket to provide a
-default implementation that passes static security scans. Users of the module
-would inherit these best practices by default, but they would still have the
-ability to configure any of the original properties on `AWS::S3::Bucket`, in
-addition to the properties defined as module parameters.
+create local modules of reuseable code that can be inserted into templates. 
+Rain modules are basically just CloudFormation templates, with a Parameters 
+section that corresponds to the Properties that a consumer will set when 
+using the module. Rain modules are very flexible, since you can override 
+any of the resource properties from the parent template.
 
 In order to use this feature, you have to acknowledge that it's experimental by
 adding a flag on the command line:
@@ -305,8 +302,7 @@ directives.
 A sample module:
 
 ```yaml
-Description: |
-  This module extends AWS::S3::Bucket
+Description: This module creates a compliant bucket, along with a second bucket to store access logs
 
 Parameters:
   LogBucketName:
@@ -421,13 +417,89 @@ Resources:
         RestrictPublicBuckets: true
 ```
 
-### Module package publishing
+### Module publishing
 
 Rain integrates with AWS CodeArtifact to enable an experience similar to npm
 publish and install. A directory that includes Rain module YAML files can be
-packaged up with `rain module publish`, and then the package can be installed
+packaged up with `rain module publish`, and then the directory can be installed
 by developers with `rain module install`.
 
+In addition to the CodeArtifact integration, you can reference a collection of 
+Rain modules with an alias inside of the parent template. Add a `Rain` section 
+to the template to configure the package alias. There's nothing special about 
+a package, it's just an alias to a directory or a zip file. A zip file can also 
+have a sha256 hash associated with it to verify the contents.
+
+```yaml
+Rain:
+  Package:
+    Location: https://github.com/aws-cloudformation/rain/modules
+    Alias: aws
+  Package:
+    Location: ./my-modules
+    Alias: xyz
+  Package:
+    Location: https://github.com/aws-cloudformation/rain/releases/tag/m0.1.0/modules.zip
+    Hash: https://github.com/aws-cloudformation/rain/releases/tag/m0.1.0/modules.sha256
+    Alias: abc
+
+Resources:
+  Foo:
+    Type: !Rain::Module aws/foo.yaml
+
+  Bar:
+    Type: !Rain::Module xyz/bar.yaml
+
+  Baz:
+    Type: $abc/baz 
+    # Shorthand for !Rain::Module abc/baz.yaml
+```
+
+A module package is published and released from this repository separately from
+the Rain binary release. This allows the package to be referenced by version
+numbers using tags, such as `m0.1.0` as shown in the example above. The major
+version number will be incremented if any breaking changes are introduced to
+the modules. The available modules in the release package are listed below. 
+
+#### simple-vpc.yaml
+
+A VPC with just two availability zones. This module is useful for POCs and simple projects.
+
+#### encrypted-bucket.yaml
+
+A simple bucket with encryption enabled and public access blocked
+
+#### compliant-bucket.yaml
+
+A bucket, plus extra buckets for access logs and replication and a bucket policy that should pass most typical compliance checks.
+
+#### bucket-policy.yaml
+
+A bucket policy that denies requests not made with TLS.
+
+#### load-balancer.yaml
+
+An ELBv2 load balancer
+
+#### static-site.yaml
+
+An S3 bucket and a CloudFront distribution to host content for a web site
+
+#### cognito.yaml
+
+A Cognito User Pool and associated resources
+
+#### rest-api.yaml
+
+An API Gateway REST API
+
+#### api-resource.yaml
+
+A Lambda function and associated API Gateway resources
+
+
+
+
 ### Gantt Chart
 
 Output a chart to an HTML file that you can view with a browser to look at how long stack operations take for each resource.
diff --git a/test/modules/api-resource.yaml b/modules/api-resource.yaml
similarity index 100%
rename from test/modules/api-resource.yaml
rename to modules/api-resource.yaml
diff --git a/test/modules/bucket-policy.yaml b/modules/bucket-policy.yaml
similarity index 56%
rename from test/modules/bucket-policy.yaml
rename to modules/bucket-policy.yaml
index 89b641aa..8dc76a5c 100644
--- a/test/modules/bucket-policy.yaml
+++ b/modules/bucket-policy.yaml
@@ -18,15 +18,4 @@ Resources:
             Resource:
               - !Sub "arn:${AWS::Partition}:s3:::${PolicyBucketName}"
               - !Sub "arn:${AWS::Partition}:s3:::${PolicyBucketName}/*"
-          - Action: s3:PutObject
-            Condition:
-              ArnLike:
-                aws:SourceArn: !Sub "arn:${AWS::Partition}:s3:::${PolicyBucketName}"
-              StringEquals:
-                aws:SourceAccount: !Ref AWS::AccountId
-            Effect: Allow
-            Principal:
-              Service: logging.s3.amazonaws.com
-            Resource:
-              - !Sub "arn:${AWS::Partition}:s3:::${PolicyBucketName}/*"
         Version: "2012-10-17"
diff --git a/test/modules/cognito.yaml b/modules/cognito.yaml
similarity index 100%
rename from test/modules/cognito.yaml
rename to modules/cognito.yaml
diff --git a/test/modules/bucket.yaml b/modules/compliant-bucket.yaml
similarity index 80%
rename from test/modules/bucket.yaml
rename to modules/compliant-bucket.yaml
index 3ff45cb2..566e0255 100644
--- a/test/modules/bucket.yaml
+++ b/modules/compliant-bucket.yaml
@@ -20,6 +20,15 @@ Parameters:
     Description: If true, the contents of all buckets will be permanently deleted when the stack is deleted.
     Default: false
 
+Rain:
+  Constants:
+    S3Arn: arn:${AWS::Partition}:s3:::
+    BucketName: ${AppName}-${AWS::Region}-${AWS::AccountId}
+    LogBucketName: ${AppName}-logs-${AWS::Region}-${AWS::AccountId}
+    LogBucketArn: ${Rain::S3Arn}${Rain::LogBucketName}
+    ReplicaBucketName: ${AppName}-replicas-${AWS::Region}-${AWS::AccountId}
+
+
 Resources:
 
   LogBucket:
@@ -42,7 +51,7 @@ Resources:
         ServerSideEncryptionConfiguration:
           - ServerSideEncryptionByDefault:
               SSEAlgorithm: AES256
-      BucketName: !Sub ${AppName}-logs-${AWS::Region}-${AWS::AccountId}
+      BucketName: !Sub ${Rain::LogBucketName}
       ObjectLockConfiguration:
         ObjectLockEnabled: Enabled
         Rule:
@@ -61,7 +70,33 @@ Resources:
   LogBucketAccess:
     Type: !Rain::Module "bucket-policy.yaml"
     Properties:
-      PolicyBucketName: !Sub ${AppName}-logs-${AWS::Region}-${AWS::AccountId}
+      PolicyBucketName: !Sub ${Rain::LogBucketName}
+    Overrides:
+      Policy:
+        Properties:
+          PolicyDocument:
+            Statement:
+              - Action: s3:*
+                Condition:
+                  Bool:
+                    aws:SecureTransport: false
+                Effect: Deny
+                Principal:
+                  AWS: '*'
+                Resource:
+                  - !Sub ${Rain::LogBucketArn}
+                  - !Sub ${Rain::LogBucketArn}/*
+              - Action: s3:PutObject
+                Condition:
+                  ArnLike:
+                    aws:SourceArn: ${Rain::LogBucketArn}/*
+                  StringEquals:
+                    aws:SourceAccount: !Ref AWS::AccountId
+                Effect: Allow
+                Principal:
+                  Service: logging.s3.amazonaws.com
+                Resource:
+                  - !Sub ${Rain::LogBucketName}/*
 
   Bucket:
     Type: AWS::S3::Bucket
@@ -78,7 +113,7 @@ Resources:
         ServerSideEncryptionConfiguration:
           - ServerSideEncryptionByDefault:
               SSEAlgorithm: AES256
-      BucketName: !Sub ${AppName}-${AWS::Region}-${AWS::AccountId}
+      BucketName: !Sub ${Rain::BucketName}
       LoggingConfiguration:
         DestinationBucketName: !Ref LogBucket
       ObjectLockEnabled: false
diff --git a/modules/encrypted-bucket.yaml b/modules/encrypted-bucket.yaml
new file mode 100644
index 00000000..6d00f126
--- /dev/null
+++ b/modules/encrypted-bucket.yaml
@@ -0,0 +1,17 @@
+Description: A simple S3 bucket with encryption enabled and public access blocked
+
+Resources:
+
+  Bucket:
+    Type: AWS::S3::Bucket
+    Properties:
+      BucketEncryption:
+        ServerSideEncryptionConfiguration:
+          - ServerSideEncryptionByDefault:
+              SSEAlgorithm: AES256
+      PublicAccessBlockConfiguration:
+        BlockPublicAcls: true
+        BlockPublicPolicy: true
+        IgnorePublicAcls: true
+        RestrictPublicBuckets: true
+
diff --git a/test/modules/load-balancer.yaml b/modules/load-balancer.yaml
similarity index 100%
rename from test/modules/load-balancer.yaml
rename to modules/load-balancer.yaml
diff --git a/test/modules/rest-api.yaml b/modules/rest-api.yaml
similarity index 100%
rename from test/modules/rest-api.yaml
rename to modules/rest-api.yaml
diff --git a/test/modules/vpc.yaml b/modules/simple-vpc.yaml
similarity index 98%
rename from test/modules/vpc.yaml
rename to modules/simple-vpc.yaml
index d0703ec0..bc8fb22e 100644
--- a/test/modules/vpc.yaml
+++ b/modules/simple-vpc.yaml
@@ -1,3 +1,5 @@
+Description: A simple VPC with two subnets
+
 Resources:
 
   VPC:
diff --git a/test/modules/static-site.yaml b/modules/static-site.yaml
similarity index 97%
rename from test/modules/static-site.yaml
rename to modules/static-site.yaml
index be884002..144e0b5e 100644
--- a/test/modules/static-site.yaml
+++ b/modules/static-site.yaml
@@ -90,7 +90,7 @@ Resources:
                 - Name: NoUserAgent_HEADER
 
   Content:
-    Type: !Rain::Module "bucket.yaml"
+    Type: !Rain::Module "compliant-bucket.yaml"
     Properties:
       AppName: !Sub ${AppName}-content
       EmptyOnDelete: true
@@ -131,7 +131,7 @@ Resources:
             Version: "2012-10-17"
 
   CloudFrontLogs:
-    Type: !Rain::Module "bucket.yaml"
+    Type: !Rain::Module "compliant-bucket.yaml"
     Properties:
       AppName: !Sub ${AppName}-cflogs
       EmptyOnDelete: true
diff --git a/scripts/integ.sh b/scripts/integ.sh
index 211a7426..ed914da6 100755
--- a/scripts/integ.sh
+++ b/scripts/integ.sh
@@ -54,6 +54,9 @@ set -eoux pipefail
 # Make sure build recommendations work
 ./internal/cmd/build/tmpl/scripts/validate.sh
 
+# Make sure modules package and lint
+./rain pkg -x --profile rain test/webapp/webapp.yaml | cfn-lint
+
 # Make sure pkl generation works
 ./rain fmt test/templates/success.template --pkl
 ./rain fmt test/templates/success.template --pkl --pkl-basic
diff --git a/test/modules/ext-ref.yaml b/test/modules/ext-ref.yaml
deleted file mode 100644
index c5bfe342..00000000
--- a/test/modules/ext-ref.yaml
+++ /dev/null
@@ -1,28 +0,0 @@
-Description: |
-  For testing Refs on the ModuleExtension itself
-
-Parameters:
-
-  RetentionPolicy:
-    Type: String
-    AllowedValues:
-      - Delete
-      - Retain
-
-Resources:
-
-  ModuleExtension:
-    Metadata:
-      Extends: AWS::S3::Bucket
-    Properties:
-      PropA: B
-      SomeProperty: !Ref RetentionPolicy
-    DeletionPolicy: !Ref RetentionPolicy
-  
-#  DependsOnModuleExtension:
-#    Type: AWS::S3::Bucket
-#    DependsOn: ModuleExtension
-#    DeletionPolicy: !Ref RetentionPolicy
-
-
-
diff --git a/test/modules/https.yaml b/test/modules/https.yaml
deleted file mode 100644
index e92fb9a8..00000000
--- a/test/modules/https.yaml
+++ /dev/null
@@ -1,6 +0,0 @@
-Resources:
-  Bucket:
-    Type: !Rain::Module "https://raw.githubusercontent.com/aws-cloudformation/rain/main/internal/cmd/build/tmpl/modules/bucket.yaml"
-    Properties:
-      AppName: hello
-
diff --git a/test/webapp/webapp.yaml b/test/webapp/webapp.yaml
index f1217846..32a8dfcd 100644
--- a/test/webapp/webapp.yaml
+++ b/test/webapp/webapp.yaml
@@ -19,7 +19,7 @@ Parameters:
 Resources:
 
   Site:
-    Type: !Rain::Module "../modules/static-site.yaml"
+    Type: !Rain::Module "../../modules/static-site.yaml"
     Properties:
       AppName: !Ref AppName
     Overrides:
@@ -41,7 +41,7 @@ Resources:
                 - Rain::OutputValue AppClientId
 
   Cognito:
-    Type: !Rain::Module "../modules/cognito.yaml"
+    Type: !Rain::Module "../../modules/cognito.yaml"
     Properties:
       AppName: !Ref AppName
       CallbackURL: !Sub "https://${SiteDistribution.DomainName}/index.html"
@@ -50,7 +50,7 @@ Resources:
         DependsOn: SiteDistribution
 
   Rest:
-    Type: !Rain::Module "../modules/rest-api.yaml"
+    Type: !Rain::Module "../../modules/rest-api.yaml"
     Properties:
       AppName: !Ref AppName
       UserPoolArn: !GetAtt CognitoUserPool.Arn
@@ -65,15 +65,15 @@ Resources:
           - JwtResourceOptions
 
   TestResource:
-    Type: !Rain::Module "../modules/api-resource.yaml"
+    Type: !Rain::Module "../../modules/api-resource.yaml"
     Metadata:
       Comment: This module handles all methods on the /test path on the API. The lambda function code is located in api/resources/test.  
     Properties:
       Name: !Sub ${AppName}-test
       RestApi: !Ref RestApi
       RestApiDeployment: !Ref RestApiDeployment
-      BuildScript: ../webapp/buildapi.sh
-      CodePath: ../webapp/api/dist/test/lambda-handler.zip
+      BuildScript: ../test/webapp/buildapi.sh
+      CodePath: ../test/webapp/api/dist/test/lambda-handler.zip
       ResourcePath: test
       StageName: staging
       AuthorizerId: !Ref RestApiAuthorizer
@@ -116,15 +116,15 @@ Resources:
           KeyType: HASH
 
   JwtResource:
-    Type: !Rain::Module "../modules/api-resource.yaml"
+    Type: !Rain::Module "../../modules/api-resource.yaml"
     Metadata:
       Comment: This module handles all methods on the /jwt path on the API. The lambda function code is located in api/resources/jwt
     Properties:
       Name: !Sub ${AppName}-jwt
       RestApi: !Ref RestApi
       RestApiDeployment: !Ref RestApiDeployment
-      BuildScript: ../webapp/buildapi.sh
-      CodePath: ../webapp/api/dist/jwt/lambda-handler.zip
+      BuildScript: ../test/webapp/buildapi.sh
+      CodePath: ../test/webapp/api/dist/jwt/lambda-handler.zip
       ResourcePath: jwt
       StageName: staging
       AuthorizerId: AWS::NoValue

From fe86512c57f516744d94e2a82edda4f9a88ec9fc Mon Sep 17 00:00:00 2001
From: Eric Beard <ericzbeard@gmail.com>
Date: Tue, 5 Nov 2024 15:04:24 -0800
Subject: [PATCH 02/15] Module package aliases

---
 .github/workflows/modules.yml    |  26 +++++++
 cft/cft.go                       |  15 ++++
 cft/pkg/module.go                |  56 ++++++++++++++-
 cft/pkg/module_test.go           |   4 ++
 cft/pkg/pkg.go                   | 120 +++++++++++++++++++++----------
 cft/pkg/tmpl/alias-expect.yaml   |   4 ++
 cft/pkg/tmpl/alias-module.yaml   |   4 ++
 cft/pkg/tmpl/alias-template.yaml |   8 +++
 docs/README.tmpl                 |  21 +++---
 internal/s11n/s11n.go            |  30 ++++++++
 internal/s11n/s11n_test.go       |  33 +++++++++
 11 files changed, 269 insertions(+), 52 deletions(-)
 create mode 100644 cft/pkg/tmpl/alias-expect.yaml
 create mode 100644 cft/pkg/tmpl/alias-module.yaml
 create mode 100644 cft/pkg/tmpl/alias-template.yaml

diff --git a/.github/workflows/modules.yml b/.github/workflows/modules.yml
index e084f4cc..174a5749 100644
--- a/.github/workflows/modules.yml
+++ b/.github/workflows/modules.yml
@@ -6,6 +6,29 @@ on:
 name: Create a module release from tag
 
 jobs:
+  build:
+    name: Build
+    runs-on: ubuntu-latest
+    defaults:
+      run:
+        shell: bash
+    container: golang:latest
+    steps:
+      - name: Checkout code
+        uses: actions/checkout@v4
+
+      - name: Install dependencies
+        run: |
+          apt-get update
+          apt-get install -y zip
+
+      - name: Build
+        run: ./.github/workflows/modules.sh 
+
+      - uses: actions/upload-artifact@v4
+        with:
+          name: dist
+          path: ./dist/*
 
   release:
     name: Release
@@ -29,6 +52,9 @@ jobs:
           for zip in ./dist/*.zip; do
             assets+=("$zip")
           done
+          for hash in ./dist/*.rsa256; do
+            assets+=("$hash")
+          done
           gh release upload "${GITHUB_REF##*/}" "${assets[@]}"
         env:
           GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
diff --git a/cft/cft.go b/cft/cft.go
index d3d1e10b..cf6c869a 100644
--- a/cft/cft.go
+++ b/cft/cft.go
@@ -14,12 +14,27 @@ import (
 	"gopkg.in/yaml.v3"
 )
 
+// PackageAlias is an alias to a module package location
+// A Rain package is a directory of modules, which are single yaml files.
+// See the main README for more
+type PackageAlias struct {
+	// Alias is a simple string like "aws"
+	Alias string
+
+	// Location is the URI where the package is stored
+	Location string
+
+	// Hash is an optional hash for zipped packages hosted on a URL
+	Hash string
+}
+
 // Template represents a CloudFormation template. The Template type
 // is minimal for now but will likely grow new features as needed by rain.
 type Template struct {
 	Node *yaml.Node
 
 	Constants map[string]*yaml.Node
+	Packages  map[string]*PackageAlias
 }
 
 // TODO - We really need a convenient Template data structure
diff --git a/cft/pkg/module.go b/cft/pkg/module.go
index 04f30270..c9738a85 100644
--- a/cft/pkg/module.go
+++ b/cft/pkg/module.go
@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"io"
 	"net/http"
+	"os"
 	"path/filepath"
 	"strings"
 
@@ -50,6 +51,7 @@ func MergeNodes(original *yaml.Node, override *yaml.Node) *yaml.Node {
 
 		return retval
 	}
+	// TODO: Not working for adding statements to a Policy
 
 	// else they are both Mapping nodes
 
@@ -901,6 +903,20 @@ func downloadModule(uri string) ([]byte, error) {
 	return content, nil
 }
 
+func checkPackageAlias(t cft.Template, uri string) *cft.PackageAlias {
+	config.Debugf("checkPackageAlias uri: %s", uri)
+	tokens := strings.Split(uri, "/")
+	if len(tokens) > 1 {
+		// See if this is one of the template package aliases
+		if t.Packages != nil {
+			if p, ok := t.Packages[tokens[0]]; ok {
+				return p
+			}
+		}
+	}
+	return nil
+}
+
 // Type: !Rain::Module
 func module(ctx *directiveContext) (bool, error) {
 
@@ -926,6 +942,15 @@ func module(ctx *directiveContext) (bool, error) {
 
 	baseUri := ctx.baseUri
 
+	// Check to see if this is an alias like "alias/foo.yaml"
+	packageAlias := checkPackageAlias(t, uri)
+	if packageAlias != nil {
+		config.Debugf("Found package alias: %+v", packageAlias)
+		uri = strings.Replace(uri, packageAlias.Alias, packageAlias.Location, 1)
+		config.Debugf("uri is now %s", uri)
+		config.Debugf("baseUri is %s", baseUri)
+	}
+
 	// Is this a local file or a URL?
 	if strings.HasPrefix(uri, "https://") {
 
@@ -941,6 +966,8 @@ func module(ctx *directiveContext) (bool, error) {
 		// Strip the file name from the uri
 		urlParts := strings.Split(uri, "/")
 		baseUri = strings.Join(urlParts[:len(urlParts)-1], "/")
+
+		// TODO: This might be a zipped directory. Validate the hash and unzip.
 	} else {
 		if baseUri != "" {
 			// If we have a base URL, prepend it to the relative path
@@ -966,7 +993,23 @@ func module(ctx *directiveContext) (bool, error) {
 			newRootDir = filepath.Dir(embeddedPath)
 		} else {
 			// Read the local file
-			content, path, err = expectFile(n, root)
+			path := uri
+			if !filepath.IsAbs(path) {
+				path = filepath.Join(root, path)
+			}
+
+			config.Debugf("abs path: %v", path)
+
+			info, err := os.Stat(path)
+			if err != nil {
+				return false, err
+			}
+
+			if info.IsDir() {
+				return false, fmt.Errorf("'%s' is a directory", path)
+			}
+
+			content, err = os.ReadFile(path)
 			if err != nil {
 				return false, err
 			}
@@ -974,6 +1017,8 @@ func module(ctx *directiveContext) (bool, error) {
 		}
 	}
 
+	// TODO: Download if it's a zip
+
 	// Parse the file
 	var moduleNode yaml.Node
 	err = yaml.Unmarshal(content, &moduleNode)
@@ -986,16 +1031,23 @@ func module(ctx *directiveContext) (bool, error) {
 		return false, err
 	}
 
+	// Figure out parent nodes to handle nested modules
 	var newParent node.NodePair
 	if parent.Parent != nil && parent.Parent.Value != nil {
 		newParent = node.GetParent(n, parent.Parent.Value, nil)
 		newParent.Parent = &parent
 	}
 
+	// Treat the module as a template
+	moduleAsTemplate := cft.Template{Node: &moduleNode}
+
+	// Read things like Constants
+	processRainSection(&moduleAsTemplate)
+
 	_, err = transform(&transformContext{
 		nodeToTransform: &moduleNode,
 		rootDir:         newRootDir,
-		t:               cft.Template{Node: &moduleNode},
+		t:               moduleAsTemplate,
 		parent:          &newParent,
 		fs:              ctx.fs,
 		baseUri:         baseUri,
diff --git a/cft/pkg/module_test.go b/cft/pkg/module_test.go
index 240169b8..581be039 100644
--- a/cft/pkg/module_test.go
+++ b/cft/pkg/module_test.go
@@ -55,6 +55,10 @@ func TestOverride(t *testing.T) {
 	runFailTest("override", t)
 }
 
+func TestPackageAlias(t *testing.T) {
+	runTest("alias", t)
+}
+
 // TODO: This was broken in the refactor, come back to it later
 //func TestForeach(t *testing.T) {
 //	runTest("foreach", t)
diff --git a/cft/pkg/pkg.go b/cft/pkg/pkg.go
index a4c7e79b..764690fc 100644
--- a/cft/pkg/pkg.go
+++ b/cft/pkg/pkg.go
@@ -79,54 +79,99 @@ func transform(ctx *transformContext) (bool, error) {
 	return changed, nil
 }
 
-// Template returns t with assets included as per AWS CLI packaging rules
-// and any Rain:: functions used.
-// rootDir must be passed in so that any included assets can be loaded from the same directory
-func Template(t cft.Template, rootDir string, fs *embed.FS) (cft.Template, error) {
-	templateNode := t.Node
-	var err error
-
-	//config.Debugf("Original template short: %v", node.ToSJson(t.Node))
-	//config.Debugf("Original template long: %v", node.ToJson(t.Node))
-
-	// First look for a Rain section and store constants
+// processRainSection returns true if the Rain section of the template existed
+// The section is removed by this function
+func processRainSection(t *cft.Template) bool {
 	t.Constants = make(map[string]*yaml.Node)
 	rainNode, err := t.GetSection(cft.Rain)
-	hasRainNode := false
 	if err != nil {
+		// This is okay, not all templates have a Rain section
 		config.Debugf("Unable to get Rain section: %v", err)
-	} else {
-		hasRainNode = true
-		config.Debugf("Rain node: %s", node.ToSJson(rainNode))
-		_, c, _ := s11n.GetMapValue(rainNode, "Constants")
-		if c != nil {
-			for i := 0; i < len(c.Content); i += 2 {
-				name := c.Content[i].Value
-				val := c.Content[i+1]
-				t.Constants[name] = val
-				// Visit each node in val looking for any ${Rain::ConstantName}
-				// and replace it with prior constant entries
-				vf := func(v *visitor.Visitor) {
-					vnode := v.GetYamlNode()
-					if vnode.Kind == yaml.ScalarNode {
-						err := replaceConstants(vnode, t.Constants)
-						if err != nil {
-							// These constants must be scalars
-							config.Debugf("replaceConstants failed: %v", err)
-						}
+		return false
+	}
+
+	config.Debugf("Rain node: %s", node.ToSJson(rainNode))
+	// Process constants in order, since they can refer to previous ones
+	_, c, _ := s11n.GetMapValue(rainNode, "Constants")
+	if c != nil {
+		for i := 0; i < len(c.Content); i += 2 {
+			name := c.Content[i].Value
+			val := c.Content[i+1]
+			t.Constants[name] = val
+			// Visit each node in val looking for any ${Rain::ConstantName}
+			// and replace it with prior constant entries
+			vf := func(v *visitor.Visitor) {
+				vnode := v.GetYamlNode()
+				if vnode.Kind == yaml.ScalarNode {
+					err := replaceConstants(vnode, t.Constants)
+					if err != nil {
+						// These constants must be scalars
+						config.Debugf("replaceConstants failed: %v", err)
 					}
 				}
-				v := visitor.NewVisitor(val)
-				v.Visit(vf)
 			}
+			v := visitor.NewVisitor(val)
+			v.Visit(vf)
 		}
+	}
 
-		// Add handling for any other features we add to the Rain section here
+	// Package aliases
+	packages := s11n.GetMap(rainNode, "Packages")
+	if packages != nil {
+		t.Packages = make(map[string]*cft.PackageAlias)
+		for k, v := range packages {
+			p := &cft.PackageAlias{}
+			p.Alias = k
+			p.Location = s11n.GetValue(v, "Location")
+			p.Hash = s11n.GetValue(v, "Hash")
+			t.Packages[k] = p
+		}
+
+		// Visit all resources to look for Type nodes that use $alias.module shorthand
+		resources, err := t.GetSection(cft.Resources)
+		if err == nil {
+			for i := 0; i < len(resources.Content); i += 2 {
+				resource := resources.Content[i+1]
+				_, typ, _ := s11n.GetMapValue(resource, "Type")
+				if typ == nil {
+					continue
+				}
+				if strings.HasPrefix(typ.Value, "$") {
+					// This is a package alias, fix it so it gets processed later
+					//typ.Value = "!Rain::Module " + strings.Trim(typ.Value, "$")
+					newTypeNode := yaml.Node{Kind: yaml.MappingNode}
+					newTypeNode.Content = []*yaml.Node{
+						&yaml.Node{Kind: yaml.ScalarNode, Value: "Rain::Module"},
+						&yaml.Node{Kind: yaml.ScalarNode, Value: strings.Trim(typ.Value, "$")},
+					}
+					*typ = newTypeNode
+				}
+			}
+		}
 
-		// Now remove the Rain node from the template
-		t.RemoveSection(cft.Rain)
 	}
 
+	// Add handling for any other features we add to the Rain section here
+
+	// Now remove the Rain node from the template
+	t.RemoveSection(cft.Rain)
+
+	return true
+}
+
+// Template returns t with assets included as per AWS CLI packaging rules
+// and any Rain:: functions used.
+// rootDir must be passed in so that any included assets can be loaded from the same directory
+func Template(t cft.Template, rootDir string, fs *embed.FS) (cft.Template, error) {
+	templateNode := t.Node
+	var err error
+
+	//config.Debugf("Original template short: %v", node.ToSJson(t.Node))
+	//config.Debugf("Original template long: %v", node.ToJson(t.Node))
+
+	// First look for a Rain section and store constants
+	hasRainSection := processRainSection(&t)
+
 	constants := t.Constants
 
 	ctx := &transformContext{
@@ -198,7 +243,8 @@ func Template(t cft.Template, rootDir string, fs *embed.FS) (cft.Template, error
 	v.Visit(replaceAnchors)
 
 	// Look for ${Rain::ConstantName} in all Sub strings
-	if hasRainNode {
+	if hasRainSection {
+		// Note that this rewrites all Subs and might have side effects
 		replaceTemplateConstants(templateNode, constants)
 	}
 
diff --git a/cft/pkg/tmpl/alias-expect.yaml b/cft/pkg/tmpl/alias-expect.yaml
new file mode 100644
index 00000000..6522457c
--- /dev/null
+++ b/cft/pkg/tmpl/alias-expect.yaml
@@ -0,0 +1,4 @@
+Resources:
+  foobar:
+    Type: AWS::S3::Bucket
+
diff --git a/cft/pkg/tmpl/alias-module.yaml b/cft/pkg/tmpl/alias-module.yaml
new file mode 100644
index 00000000..d9cd0ae9
--- /dev/null
+++ b/cft/pkg/tmpl/alias-module.yaml
@@ -0,0 +1,4 @@
+Resources:
+  bar:
+    Type: AWS::S3::Bucket
+
diff --git a/cft/pkg/tmpl/alias-template.yaml b/cft/pkg/tmpl/alias-template.yaml
new file mode 100644
index 00000000..7c398df3
--- /dev/null
+++ b/cft/pkg/tmpl/alias-template.yaml
@@ -0,0 +1,8 @@
+Rain:
+  Packages:
+    abc:
+      Location: .
+Resources:
+  foo:
+    Type: $abc/alias-module.yaml
+
diff --git a/docs/README.tmpl b/docs/README.tmpl
index 2ed575f2..01301e88 100644
--- a/docs/README.tmpl
+++ b/docs/README.tmpl
@@ -432,16 +432,14 @@ have a sha256 hash associated with it to verify the contents.
 
 ```yaml
 Rain:
-  Package:
-    Location: https://github.com/aws-cloudformation/rain/modules
-    Alias: aws
-  Package:
-    Location: ./my-modules
-    Alias: xyz
-  Package:
-    Location: https://github.com/aws-cloudformation/rain/releases/tag/m0.1.0/modules.zip
-    Hash: https://github.com/aws-cloudformation/rain/releases/tag/m0.1.0/modules.sha256
-    Alias: abc
+  Packages:
+    aws:
+      Location: https://github.com/aws-cloudformation/rain/modules
+    xyz:
+      Location: ./my-modules
+    abc:
+      Location: https://github.com/aws-cloudformation/rain/releases/tag/m0.1.0/modules.zip
+      Hash: https://github.com/aws-cloudformation/rain/releases/tag/m0.1.0/modules.sha256
 
 Resources:
   Foo:
@@ -497,9 +495,6 @@ An API Gateway REST API
 
 A Lambda function and associated API Gateway resources
 
-
-
-
 ### Gantt Chart
 
 Output a chart to an HTML file that you can view with a browser to look at how long stack operations take for each resource.
diff --git a/internal/s11n/s11n.go b/internal/s11n/s11n.go
index 1165205a..e09de0dd 100644
--- a/internal/s11n/s11n.go
+++ b/internal/s11n/s11n.go
@@ -39,6 +39,36 @@ func GetMapValue(n *yaml.Node, key string) (*yaml.Node, *yaml.Node, error) {
 	return nil, nil, fmt.Errorf("key %s not found", key)
 }
 
+// GetValue tries to get a scalar value from a mapping node
+// If anything goes wrong, it returns an empty string.
+// Use GetMapValue if you need more control.
+func GetValue(n *yaml.Node, name string) string {
+	_, v, _ := GetMapValue(n, name)
+	if v != nil && v.Kind == yaml.ScalarNode {
+		return v.Value
+	}
+	return ""
+}
+
+// GetMap returns a map of the names and values in a MappingNode
+// This is a shorthand version of calling GetMapValue and iterating over the results.
+// One difference is that keys will no longer be in their original order,
+// which matters for several use cases like processing Constants.
+func GetMap(n *yaml.Node, name string) map[string]*yaml.Node {
+	_, c, _ := GetMapValue(n, name)
+	if c == nil {
+		return nil
+	}
+	retval := make(map[string]*yaml.Node)
+
+	for i := 0; i < len(c.Content); i += 2 {
+		k := c.Content[i].Value
+		v := c.Content[i+1]
+		retval[k] = v
+	}
+	return retval
+}
+
 // GetPath returns the node by descending into map and array nodes for each element of path
 func GetPath(node *yaml.Node, path []interface{}) (*yaml.Node, error) {
 	if node.Kind == yaml.DocumentNode {
diff --git a/internal/s11n/s11n_test.go b/internal/s11n/s11n_test.go
index 8a975426..8cc36113 100644
--- a/internal/s11n/s11n_test.go
+++ b/internal/s11n/s11n_test.go
@@ -48,3 +48,36 @@ func TestGetNodePath(t *testing.T) {
 		}
 	}
 }
+
+func TestGetMap(t *testing.T) {
+	var base yaml.Node
+	err := yaml.Unmarshal([]byte(nodeTestBase), &base)
+	if err != nil {
+		t.Fatal(err)
+	}
+	n := base.Content[0]
+	m := s11n.GetMap(n, "baz")
+	if m == nil {
+		t.Fatal("expected baz map")
+	}
+	if v, ok := m["quux"]; ok {
+		if v.Value != "mooz" {
+			t.Fatalf("expected baaz.quux to be mooz")
+		}
+	} else {
+		t.Fatalf("expected to find baaz.quux")
+	}
+}
+
+func TestGetValue(t *testing.T) {
+	var base yaml.Node
+	err := yaml.Unmarshal([]byte(nodeTestBase), &base)
+	if err != nil {
+		t.Fatal(err)
+	}
+	n := base.Content[0]
+	v := s11n.GetValue(n, "foo")
+	if v != "bar" {
+		t.Fatal("expected foo: bar")
+	}
+}

From 425fa1ad8a451f73952ccbd17dc66013fada8fca Mon Sep 17 00:00:00 2001
From: Eric Beard <ericzbeard@gmail.com>
Date: Wed, 6 Nov 2024 16:59:42 -0800
Subject: [PATCH 03/15] Add IfParam modulel conditional

---
 cft/pkg/module.go                  | 40 ++++++++++++++++++++++++--
 cft/pkg/module_test.go             |  4 +++
 cft/pkg/tmpl/ifparam-expect.yaml   |  6 ++++
 cft/pkg/tmpl/ifparam-module.yaml   | 11 ++++++++
 cft/pkg/tmpl/ifparam-template.yaml |  7 +++++
 modules/simple-table.yaml          | 45 ++++++++++++++++++++++++++++++
 modules/static-site.yaml           | 11 --------
 test/webapp/webapp.yaml            | 31 ++------------------
 8 files changed, 113 insertions(+), 42 deletions(-)
 create mode 100644 cft/pkg/tmpl/ifparam-expect.yaml
 create mode 100644 cft/pkg/tmpl/ifparam-module.yaml
 create mode 100644 cft/pkg/tmpl/ifparam-template.yaml
 create mode 100644 modules/simple-table.yaml

diff --git a/cft/pkg/module.go b/cft/pkg/module.go
index c9738a85..1a12145f 100644
--- a/cft/pkg/module.go
+++ b/cft/pkg/module.go
@@ -704,6 +704,29 @@ func processModule(
 			continue
 		}
 		name := moduleResources.Content[i-1].Value
+
+		// Check to see if there is a Rain attribute in the Metadata.
+		// If so, check conditionals like IfParam
+		metadata := s11n.GetMap(moduleResource, "Metadata")
+		if metadata != nil {
+			if rainMetadata, ok := metadata["Rain"]; ok {
+				config.Debugf("Found Metadata Rain in %s: %v", name, node.ToSJson(rainMetadata))
+				ifParam := s11n.GetValue(rainMetadata, "IfParam")
+				// If the value of IfParam is not set, omit this resource
+				if ifParam != "" {
+					if moduleParams != nil &&
+						s11n.GetMap(moduleParams, ifParam) != nil &&
+						s11n.GetValue(templateProps, ifParam) == "" {
+						config.Debugf("Omitting %s", name)
+						continue
+					} else {
+						// Get rid of the IfParam, since it's irrelevant in the resulting template
+						node.RemoveFromMap(rainMetadata, "IfParam")
+					}
+				}
+			}
+		}
+
 		nameNode := node.Clone(moduleResources.Content[i-1])
 		nameNode.Value = rename(logicalId, nameNode.Value)
 		outputNode.Content = append(outputNode.Content, nameNode)
@@ -944,15 +967,27 @@ func module(ctx *directiveContext) (bool, error) {
 
 	// Check to see if this is an alias like "alias/foo.yaml"
 	packageAlias := checkPackageAlias(t, uri)
+	isZip := false
 	if packageAlias != nil {
 		config.Debugf("Found package alias: %+v", packageAlias)
 		uri = strings.Replace(uri, packageAlias.Alias, packageAlias.Location, 1)
 		config.Debugf("uri is now %s", uri)
 		config.Debugf("baseUri is %s", baseUri)
+
+		// This might be a zipped directory.
+		if strings.HasSuffix(uri, ".zip") {
+			// Unzip, verify hash if there is one, and put the files in memory
+			isZip = true
+			// TODO
+		}
 	}
 
-	// Is this a local file or a URL?
-	if strings.HasPrefix(uri, "https://") {
+	// Is this a local file or a URL or an in memory file system?
+	if isZip {
+
+		// TODO - Get content from in memory unzipped files
+
+	} else if strings.HasPrefix(uri, "https://") {
 
 		content, err = downloadModule(uri)
 		if err != nil {
@@ -967,7 +1002,6 @@ func module(ctx *directiveContext) (bool, error) {
 		urlParts := strings.Split(uri, "/")
 		baseUri = strings.Join(urlParts[:len(urlParts)-1], "/")
 
-		// TODO: This might be a zipped directory. Validate the hash and unzip.
 	} else {
 		if baseUri != "" {
 			// If we have a base URL, prepend it to the relative path
diff --git a/cft/pkg/module_test.go b/cft/pkg/module_test.go
index 581be039..68aa7d7d 100644
--- a/cft/pkg/module_test.go
+++ b/cft/pkg/module_test.go
@@ -59,6 +59,10 @@ func TestPackageAlias(t *testing.T) {
 	runTest("alias", t)
 }
 
+func TestIfPAram(t *testing.T) {
+	runTest("ifparam", t)
+}
+
 // TODO: This was broken in the refactor, come back to it later
 //func TestForeach(t *testing.T) {
 //	runTest("foreach", t)
diff --git a/cft/pkg/tmpl/ifparam-expect.yaml b/cft/pkg/tmpl/ifparam-expect.yaml
new file mode 100644
index 00000000..f6f17b36
--- /dev/null
+++ b/cft/pkg/tmpl/ifparam-expect.yaml
@@ -0,0 +1,6 @@
+Resources:
+  ShowBucket:
+    Type: AWS::S3::Bucket
+    Metadata:
+      Rain: {}
+
diff --git a/cft/pkg/tmpl/ifparam-module.yaml b/cft/pkg/tmpl/ifparam-module.yaml
new file mode 100644
index 00000000..9a0b21a9
--- /dev/null
+++ b/cft/pkg/tmpl/ifparam-module.yaml
@@ -0,0 +1,11 @@
+Parameters:
+  Show:
+    Type: String
+
+Resources:
+  Bucket:
+    Type: AWS::S3::Bucket
+    Metadata:
+      Rain:
+        IfParam: Show
+
diff --git a/cft/pkg/tmpl/ifparam-template.yaml b/cft/pkg/tmpl/ifparam-template.yaml
new file mode 100644
index 00000000..371b1f1f
--- /dev/null
+++ b/cft/pkg/tmpl/ifparam-template.yaml
@@ -0,0 +1,7 @@
+Resources:
+  Show:
+    Type: !Rain::Module ./ifparam-module.yaml
+    Properties:
+      Show: true
+  Hide:
+    Type: !Rain::Module ./ifparam-module.yaml
diff --git a/modules/simple-table.yaml b/modules/simple-table.yaml
new file mode 100644
index 00000000..089dc15a
--- /dev/null
+++ b/modules/simple-table.yaml
@@ -0,0 +1,45 @@
+Properties:
+
+  TableName:
+    Type: String
+  
+  LambdaRoleArn:
+    Type: String
+    Description: If set, allow the lambda function to access this table
+
+Resources:
+  Table:
+    Type: AWS::DynamoDB::Table
+    Properties:
+      BillingMode: PAY_PER_REQUEST
+      TableName: !Sub ${TableName}
+      AttributeDefinitions:
+        - AttributeName: id
+          AttributeType: S
+      KeySchema:
+        - AttributeName: id
+          KeyType: HASH
+
+  LambdaPolicy:
+    Type: AWS::IAM::RolePolicy
+    Metadata:
+      Comment: This resource is created only if the LambdaRoleArn is set
+      Rain:
+        IfParam: LambdaRoleArn 
+    Properties:
+      PolicyDocument:
+        Statement:
+          - Action:
+              - dynamodb:BatchGetItem
+              - dynamodb:GetItem
+              - dynamodb:Query
+              - dynamodb:Scan
+              - dynamodb:BatchWriteItem
+              - dynamodb:PutItem
+              - dynamodb:UpdateItem
+            Effect: Allow
+            Resource: 
+              - !GetAtt Table.Arn
+      PolicyName: !Sub ${TableName}-policy
+      RoleName: !Ref LambdaRoleArn
+
diff --git a/modules/static-site.yaml b/modules/static-site.yaml
index 144e0b5e..bd6eaa27 100644
--- a/modules/static-site.yaml
+++ b/modules/static-site.yaml
@@ -117,17 +117,6 @@ Resources:
                 Resource:
                   - !Sub arn:${AWS::Partition}:s3:::${AppName}-${AWS::Region}-${AWS::AccountId}
                   - !Sub arn:${AWS::Partition}:s3:::${AppName}-${AWS::Region}-${AWS::AccountId}/*
-              - Action: s3:PutObject
-                Condition:
-                  ArnLike:
-                    aws:SourceArn: !Sub arn:${AWS::Partition}:s3:::${AppName}-${AWS::Region}-${AWS::AccountId}
-                  StringEquals:
-                    aws:SourceAccount: !Ref AWS::AccountId
-                Effect: Allow
-                Principal:
-                  Service: logging.s3.amazonaws.com
-                Resource:
-                  - !Sub arn:${AWS::Partition}:s3:::${AppName}-${AWS::Region}-${AWS::AccountId}/*
             Version: "2012-10-17"
 
   CloudFrontLogs:
diff --git a/test/webapp/webapp.yaml b/test/webapp/webapp.yaml
index 32a8dfcd..1ab155b5 100644
--- a/test/webapp/webapp.yaml
+++ b/test/webapp/webapp.yaml
@@ -84,36 +84,11 @@ Resources:
             Variables:
               TABLE_NAME: !Ref TestTable
 
-  TestResourceHandlerPolicy:
-    Type: AWS::IAM::RolePolicy
+  TestData:
+    Type: !Rain::Module "../../modules/simple-table.yaml" 
     Properties:
-      PolicyDocument:
-        Statement:
-          - Action:
-              - dynamodb:BatchGetItem
-              - dynamodb:GetItem
-              - dynamodb:Query
-              - dynamodb:Scan
-              - dynamodb:BatchWriteItem
-              - dynamodb:PutItem
-              - dynamodb:UpdateItem
-            Effect: Allow
-            Resource: 
-              - !GetAtt TestTable.Arn
-      PolicyName: handler-policy
-      RoleName: !Ref TestResourceHandlerRole
-
-  TestTable:
-    Type: AWS::DynamoDB::Table
-    Properties:
-      BillingMode: PAY_PER_REQUEST
       TableName: !Sub ${AppName}-test
-      AttributeDefinitions:
-        - AttributeName: id
-          AttributeType: S
-      KeySchema:
-        - AttributeName: id
-          KeyType: HASH
+      LambdaRoleArn: !GetAtt TestResourceHandlerRole.Arn 
 
   JwtResource:
     Type: !Rain::Module "../../modules/api-resource.yaml"

From 1dba691bc377df4d3b1c8b60b65fb4845c113232 Mon Sep 17 00:00:00 2001
From: Eric Beard <ericzbeard@gmail.com>
Date: Thu, 7 Nov 2024 09:10:09 -0800
Subject: [PATCH 04/15] Fix double document node

---
 cft/cft.go        |  2 ++
 cft/pkg/module.go |  1 +
 cft/pkg/pkg.go    | 11 +++++++----
 3 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/cft/cft.go b/cft/cft.go
index cf6c869a..b200a3a7 100644
--- a/cft/cft.go
+++ b/cft/cft.go
@@ -97,6 +97,7 @@ func (t Template) GetParameter(name string) (*yaml.Node, error) {
 func (t Template) GetNode(section Section, name string) (*yaml.Node, error) {
 	_, resMap, _ := s11n.GetMapValue(t.Node.Content[0], string(section))
 	if resMap == nil {
+		config.Debugf("GetNode t.Node: %s", node.ToSJson(t.Node))
 		return nil, fmt.Errorf("unable to locate the %s node", section)
 	}
 	// TODO: Some Sections are not Maps
@@ -142,6 +143,7 @@ func (t Template) GetSection(section Section) (*yaml.Node, error) {
 	}
 	_, s, _ := s11n.GetMapValue(t.Node.Content[0], string(section))
 	if s == nil {
+		config.Debugf("GetSection t.Node: %s", node.ToSJson(t.Node))
 		return nil, fmt.Errorf("unable to locate the %s node", section)
 	}
 	return s, nil
diff --git a/cft/pkg/module.go b/cft/pkg/module.go
index 1a12145f..a87fe935 100644
--- a/cft/pkg/module.go
+++ b/cft/pkg/module.go
@@ -722,6 +722,7 @@ func processModule(
 					} else {
 						// Get rid of the IfParam, since it's irrelevant in the resulting template
 						node.RemoveFromMap(rainMetadata, "IfParam")
+						// TODO: If there's nothing left in the Rain or Metadata, get rid of them too
 					}
 				}
 			}
diff --git a/cft/pkg/pkg.go b/cft/pkg/pkg.go
index 764690fc..99dd0b52 100644
--- a/cft/pkg/pkg.go
+++ b/cft/pkg/pkg.go
@@ -260,11 +260,14 @@ func Template(t cft.Template, rootDir string, fs *embed.FS) (cft.Template, error
 		return t, fmt.Errorf("failed to unmarshal template: %v", err)
 	}
 
-	// We lose the Document node here
-	// TODO: Actually we're ending up with 2 document nodes somehow...
+	// We might lose the Document node here
 	retval := cft.Template{}
-	retval.Node = &yaml.Node{Kind: yaml.DocumentNode, Content: make([]*yaml.Node, 0)}
-	retval.Node.Content = append(retval.Node.Content, templateNode)
+	if templateNode.Kind == yaml.DocumentNode {
+		retval.Node = templateNode
+	} else {
+		retval.Node = &yaml.Node{Kind: yaml.DocumentNode, Content: make([]*yaml.Node, 0)}
+		retval.Node.Content = append(retval.Node.Content, templateNode)
+	}
 
 	return retval, err
 }

From b72596ae8c96f59fa2ae22d7f29edf36569fd3a4 Mon Sep 17 00:00:00 2001
From: Eric Beard <ericzbeard@gmail.com>
Date: Thu, 7 Nov 2024 11:10:52 -0800
Subject: [PATCH 05/15] Fixed Constants in modules

---
 cft/pkg/module.go                   | 275 ++++++++--------------------
 cft/pkg/module_test.go              |  38 +---
 cft/pkg/tmpl/constant-expect.yaml   |   6 +
 cft/pkg/tmpl/constant-module.yaml   |  10 +
 cft/pkg/tmpl/constant-template.yaml |   4 +
 cft/pkg/tmpl/ifparam-expect.yaml    |   2 -
 internal/node/node.go               |  89 +++++++++
 internal/node/node_test.go          |  34 ++++
 modules/compliant-bucket.yaml       |  10 +-
 9 files changed, 231 insertions(+), 237 deletions(-)
 create mode 100644 cft/pkg/tmpl/constant-expect.yaml
 create mode 100644 cft/pkg/tmpl/constant-module.yaml
 create mode 100644 cft/pkg/tmpl/constant-template.yaml

diff --git a/cft/pkg/module.go b/cft/pkg/module.go
index a87fe935..74a447f5 100644
--- a/cft/pkg/module.go
+++ b/cft/pkg/module.go
@@ -18,94 +18,20 @@ import (
 	"gopkg.in/yaml.v3"
 )
 
-// mergeNodes merges two mapping nodes, replacing any values found in override
-func MergeNodes(original *yaml.Node, override *yaml.Node) *yaml.Node {
-
-	// If the nodes are not the same kind, just return the override
-	if override.Kind != original.Kind {
-		return override
-	}
-
-	if override.Kind == yaml.ScalarNode {
-		return override
-	}
-
-	retval := &yaml.Node{Kind: override.Kind, Content: make([]*yaml.Node, 0)}
-	overrideMap := make(map[string]bool)
-
-	if override.Kind == yaml.SequenceNode {
-		retval.Content = append(retval.Content, override.Content...)
-
-		for _, n := range original.Content {
-			already := false
-			for _, r := range retval.Content {
-				if r.Value == n.Value {
-					already = true
-					break
-				}
-			}
-			if !already {
-				retval.Content = append(retval.Content, n)
-			}
-		}
-
-		return retval
-	}
-	// TODO: Not working for adding statements to a Policy
-
-	// else they are both Mapping nodes
-
-	// Add everything in the override Mapping
-	for i, n := range override.Content {
-		retval.Content = append(retval.Content, n)
-		var name string
-		if i%2 == 0 {
-			// Remember what we added
-			name = n.Value
-			overrideMap[name] = true
-		} else {
-			name = retval.Content[i-1].Value
-		}
-
-		/*
-			Original:
-				A: something
-				B:
-					foo: 1
-					bar: 2
-
-			Override:
-				A: something else
-				B:
-					foo: 3
-					baz: 6
-		*/
-
-		// Recurse if this is a mapping node
-		if i%2 == 1 && n.Kind == yaml.MappingNode {
-			// Find the matching node in original
-			for j, match := range original.Content {
-				if j%2 == 0 && match.Value == name {
-					n.Content[i] = MergeNodes(n.Content[i], original.Content[j])
-				}
-			}
-		}
-	}
-
-	// Only add things from the original if they weren't in original
-	for i := 0; i < len(original.Content); i++ {
-		n := original.Content[i]
-		if i%2 == 0 {
-			if _, exists := overrideMap[n.Value]; exists {
-				i = i + 1 // Skip the next node
-				continue
-			}
-		}
-		retval.Content = append(retval.Content, n)
-	}
-
-	return retval
-}
+const (
+	Rain                = "Rain"
+	Metadata            = "Metadata"
+	IfParam             = "IfParam"
+	Overrides           = "Overrides"
+	DependsOn           = "DependsOn"
+	Properties          = "Properties"
+	CreationPolicy      = "CreationPolicy"
+	UpdatePolicy        = "UpdatePolicy"
+	DeletionPolicy      = "DeletionPolicy"
+	UpdateReplacePolicy = "UpdateReplacePolicy"
+	Condition           = "Condition"
+	Default             = "Default"
+)
 
 // Clone a property-like node from the module and replace any overridden values
 func cloneAndReplaceProps(
@@ -150,35 +76,14 @@ func cloneAndReplaceProps(
 				}
 			}
 
-			// Override anything hard coded into the module that is present in the parent template
+			// Overwrite anything hard coded into the module that is
+			// present in the parent template
 			for j, mprop := range props.Content {
-				// Property names are even-indexed array elements
 				if tprop.Value == mprop.Value && i%2 == 0 && j%2 == 0 {
-					// It's was not possible to override just one nested property
-					//
-					// Bucket:
-					//   Metadata:
-					//     Rain:
-					//       Content: site
-					//       DistributionLogicalId: AWS::NoValue
-					//
-					// Override:
-					//   Bucket:
-					//     Metadata:
-					//       Rain:
-					//         DistributionLogicalId: MyDistribution
-					//
-					// The result is that the Content node is removed
-					//
-
-					// The old way
-					// props.Content[j+1] = node.Clone(templateProps.Content[i+1])
-
-					// The new way
 					clonedNode := node.Clone(templateProps.Content[i+1])
 					// config.Debugf("original: %s", node.ToSJson(templateProps.Content[i+1]))
 					// config.Debugf("clonedNode: %s", node.ToSJson(clonedNode))
-					merged := MergeNodes(props.Content[j+1], clonedNode)
+					merged := node.MergeNodes(props.Content[j+1], clonedNode)
 					// config.Debugf("merged: %s", node.ToSJson(merged))
 					props.Content[j+1] = merged
 
@@ -243,6 +148,9 @@ type refctx struct {
 	// Template property overrides map for the resource
 	// TODO: Not necessary? We don't look anything up here...
 	overrides *yaml.Node
+
+	// The module's Constants from the Rain section
+	constants map[string]*yaml.Node
 }
 
 func replaceProp(prop *yaml.Node, parentName string, v *yaml.Node, outNode *yaml.Node, sidx int) error {
@@ -265,9 +173,6 @@ func replaceProp(prop *yaml.Node, parentName string, v *yaml.Node, outNode *yaml
 		}
 		return nil
 	}
-	// TODO Is the above good enough for the below?
-	// TODO: It doesn't work when a map needs to be replaced.
-	// But the below relies on a parentName
 
 	// We can't just set prop.Value, since we would end up with
 	// Prop: !Ref Value instead of just Prop: Value. Get the
@@ -338,7 +243,7 @@ func resolveModuleRef(parentName string, prop *yaml.Node, sidx int, ctx *refctx)
 				// Check to see if there is a Default
 				_, mParam, _ := s11n.GetMapValue(moduleParams, prop.Value)
 				if mParam != nil {
-					_, defaultNode, _ := s11n.GetMapValue(mParam, "Default")
+					_, defaultNode, _ := s11n.GetMapValue(mParam, Default)
 					if defaultNode != nil {
 						parentVal = defaultNode
 					}
@@ -473,6 +378,22 @@ func resolveModuleSub(parentName string, prop *yaml.Node, sidx int, ctx *refctx)
 			}
 			sub += fmt.Sprintf("${%s.%s}", left, right)
 			needSub = true
+		case parse.RAIN:
+			// Replace ${Rain::ConstantName} with template constant value
+			if ctx.constants == nil {
+				return fmt.Errorf("no Rain Constants section, looking for %s", word.W)
+			}
+			if c, ok := ctx.constants[word.W]; ok {
+				sub += c.Value
+			} else {
+				if len(ctx.constants) == 0 {
+					config.Debugf("Constants are empty")
+				}
+				for k, v := range ctx.constants {
+					config.Debugf("Constant %s: %s", k, v.Value)
+				}
+				return fmt.Errorf("unable to find Rain constant %s", word.W)
+			}
 		default:
 			return fmt.Errorf("unexpected word type %v for %s", word.T, word.W)
 		}
@@ -573,7 +494,7 @@ func resolveRefs(ctx *refctx) error {
 
 	// Replace references to the module's parameters with the value supplied
 	// by the parent template. Rename refs to other resources in the module.
-	propLikes := []string{"Properties", "Metadata"}
+	propLikes := []string{Properties, Metadata}
 	for _, propLike := range propLikes {
 		_, outNodeProps, _ := s11n.GetMapValue(outNode, propLike)
 		if outNodeProps != nil {
@@ -591,7 +512,7 @@ func resolveRefs(ctx *refctx) error {
 	}
 
 	// DeletionPolicy, UpdateReplacePolicy, Condition
-	policies := []string{"DeletionPolicy", "UpdateReplacePolicy", "Condition"}
+	policies := []string{DeletionPolicy, UpdateReplacePolicy, Condition}
 	for _, policy := range policies {
 		_, policyNode, _ := s11n.GetMapValue(outNode, policy)
 		if policyNode != nil {
@@ -611,7 +532,8 @@ func processModule(
 	outputNode *yaml.Node,
 	t cft.Template,
 	typeNode *yaml.Node,
-	parent node.NodePair) (bool, error) {
+	parent node.NodePair,
+	moduleConstants map[string]*yaml.Node) (bool, error) {
 
 	// The parent arg is the map in the template resource's Content[1] that contains Type, Properties, etc
 
@@ -644,10 +566,10 @@ func processModule(
 	templateResource := parent.Value // The !!map node of the resource with Type !Rain::Module
 
 	// Properties are the args that match module params
-	_, templateProps, _ := s11n.GetMapValue(templateResource, "Properties")
+	_, templateProps, _ := s11n.GetMapValue(templateResource, Properties)
 
 	// Overrides have overridden values for module resources. Anything in a module can be overridden.
-	_, overrides, _ := s11n.GetMapValue(templateResource, "Overrides")
+	_, overrides, _ := s11n.GetMapValue(templateResource, Overrides)
 
 	// Validate that the overrides actually exist and error if not
 	if overrides != nil {
@@ -674,7 +596,7 @@ func processModule(
 			// It is an error to try to override a property that shares
 			// a name with a module Parameter.
 			if moduleParams != nil {
-				_, overrideProps, _ := s11n.GetMapValue(overrides.Content[i+1], "Properties")
+				_, overrideProps, _ := s11n.GetMapValue(overrides.Content[i+1], Properties)
 				if overrideProps != nil {
 					for op, overrideProp := range overrideProps.Content {
 						if op%2 != 0 {
@@ -707,22 +629,26 @@ func processModule(
 
 		// Check to see if there is a Rain attribute in the Metadata.
 		// If so, check conditionals like IfParam
-		metadata := s11n.GetMap(moduleResource, "Metadata")
+		metadata := s11n.GetMap(moduleResource, Metadata)
 		if metadata != nil {
-			if rainMetadata, ok := metadata["Rain"]; ok {
-				config.Debugf("Found Metadata Rain in %s: %v", name, node.ToSJson(rainMetadata))
-				ifParam := s11n.GetValue(rainMetadata, "IfParam")
+			if rainMetadata, ok := metadata[Rain]; ok {
+				ifp := s11n.GetValue(rainMetadata, IfParam)
 				// If the value of IfParam is not set, omit this resource
-				if ifParam != "" {
+				if ifp != "" {
 					if moduleParams != nil &&
-						s11n.GetMap(moduleParams, ifParam) != nil &&
-						s11n.GetValue(templateProps, ifParam) == "" {
-						config.Debugf("Omitting %s", name)
+						s11n.GetMap(moduleParams, ifp) != nil &&
+						s11n.GetValue(templateProps, ifp) == "" {
 						continue
 					} else {
 						// Get rid of the IfParam, since it's irrelevant in the resulting template
-						node.RemoveFromMap(rainMetadata, "IfParam")
-						// TODO: If there's nothing left in the Rain or Metadata, get rid of them too
+						node.RemoveFromMap(rainMetadata, IfParam)
+						if len(rainMetadata.Content) == 0 {
+							_, metadataNode, _ := s11n.GetMapValue(moduleResource, Metadata)
+							node.RemoveFromMap(metadataNode, Rain)
+							if len(metadataNode.Content) == 0 {
+								node.RemoveFromMap(moduleResource, Metadata)
+							}
+						}
 					}
 				}
 			}
@@ -740,7 +666,7 @@ func processModule(
 		}
 
 		// Clone attributes that are like Properties, and replace overridden values
-		propLike := []string{"Properties", "CreationPolicy", "Metadata", "UpdatePolicy"}
+		propLike := []string{Properties, CreationPolicy, Metadata, UpdatePolicy}
 		for _, pl := range propLike {
 			_, plProps, _ := s11n.GetMapValue(moduleResource, pl)
 			_, plTemplateProps, _ := s11n.GetMapValue(templateOverrides, pl)
@@ -758,23 +684,23 @@ func processModule(
 		}
 
 		// DeletionPolicy
-		addScalarAttribute(clonedResource, "DeletionPolicy", moduleResource, templateOverrides)
+		addScalarAttribute(clonedResource, DeletionPolicy, moduleResource, templateOverrides)
 
 		// UpdateReplacePolicy
-		addScalarAttribute(clonedResource, "UpdateReplacePolicy", moduleResource, templateOverrides)
+		addScalarAttribute(clonedResource, UpdateReplacePolicy, moduleResource, templateOverrides)
 
 		// Condition
-		addScalarAttribute(clonedResource, "Condition", moduleResource, templateOverrides)
+		addScalarAttribute(clonedResource, Condition, moduleResource, templateOverrides)
 
 		// DependsOn is an array of scalars or a single scalar
-		_, moduleDependsOn, _ := s11n.GetMapValue(moduleResource, "DependsOn")
-		_, templateDependsOn, _ := s11n.GetMapValue(templateOverrides, "DependsOn")
+		_, moduleDependsOn, _ := s11n.GetMapValue(moduleResource, DependsOn)
+		_, templateDependsOn, _ := s11n.GetMapValue(templateOverrides, DependsOn)
 		if moduleDependsOn != nil || templateDependsOn != nil {
-			clonedResource.Content = append(clonedResource.Content, &yaml.Node{Kind: yaml.ScalarNode, Value: "DependsOn"})
+			clonedResource.Content = append(clonedResource.Content, &yaml.Node{Kind: yaml.ScalarNode, Value: DependsOn})
 			dependsOnValue := &yaml.Node{Kind: yaml.SequenceNode, Content: make([]*yaml.Node, 0)}
 			if moduleDependsOn != nil {
 				// Remove the original DependsOn, so we don't end up with two
-				node.RemoveFromMap(clonedResource, "DependsOn")
+				node.RemoveFromMap(clonedResource, DependsOn)
 
 				// Change the names to the modified resource name for the template
 				c := make([]*yaml.Node, 0)
@@ -809,9 +735,9 @@ func processModule(
 
 		/*
 			// Add the Condition from the parent template
-			_, parentCondition, _ := s11n.GetMapValue(templateOverrides, "Condition")
+			_, parentCondition, _ := s11n.GetMapValue(templateOverrides, Condition)
 			if parentCondition != nil {
-				clonedResource.Content = append(clonedResource.Content, &yaml.Node{Kind: yaml.ScalarNode, Value: "Condition"})
+				clonedResource.Content = append(clonedResource.Content, &yaml.Node{Kind: yaml.ScalarNode, Value: Condition})
 				clonedResource.Content = append(clonedResource.Content, node.Clone(parentCondition))
 			}
 		*/
@@ -826,6 +752,7 @@ func processModule(
 			logicalId:       logicalId,
 			moduleResources: moduleResources,
 			overrides:       templateOverrides,
+			constants:       moduleConstants,
 		}
 		err := resolveRefs(ctx)
 		if err != nil {
@@ -848,57 +775,6 @@ func processModule(
 
 		}
 
-		/*
-			// Resolve Conditions. Rain handles this differently, since a rain
-			// module cannot have a Condition section. This value must be a module parameter
-			// name, and the value must be set in the parent template as the name of
-			// a Condition that is defined in the parent.
-			_, condition, _ := s11n.GetMapValue(moduleResource, "Condition")
-			if condition != nil {
-				conditionErr := errors.New("a Condition in a rain module must be the name of " +
-					"a Parameter that is set the name of a Condition in the parent template")
-				// The value must be present in the module's parameters
-				if condition.Kind != yaml.ScalarNode {
-					return false, conditionErr
-				}
-				_, param, _ := s11n.GetMapValue(moduleParams, condition.Value)
-				if param == nil {
-					return false, conditionErr
-				}
-				_, conditionVal, _ := s11n.GetMapValue(templateProps, condition.Value)
-				if conditionVal == nil {
-					return false, conditionErr
-				}
-				if conditionVal.Kind != yaml.ScalarNode {
-					return false, conditionErr
-				}
-				condition.Value = conditionVal.Value
-			}
-		*/
-
-		/*
-			// Modify referenced resource names
-			_, dependsOn, _ := s11n.GetMapValue(clonedResource, "DependsOn")
-			if dependsOn != nil {
-				replaceDependsOn := &yaml.Node{Kind: yaml.SequenceNode, Content: make([]*yaml.Node, 0)}
-				if dependsOn.Kind == yaml.ScalarNode {
-					for _, v := range strings.Split(dependsOn.Value, " ") {
-						replaceDependsOn.Content = append(replaceDependsOn.Content,
-							&yaml.Node{Kind: yaml.ScalarNode, Value: rename(logicalId, v)})
-					}
-				} else {
-					for _, c := range dependsOn.Content {
-						for _, v := range strings.Split(c.Value, " ") {
-							replaceDependsOn.Content = append(replaceDependsOn.Content,
-								&yaml.Node{Kind: yaml.ScalarNode, Value: rename(logicalId, v)})
-						}
-					}
-				}
-				node.SetMapValue(clonedResource, "DependsOn", replaceDependsOn)
-			}
-		*/
-
-		//resolveRefs(clonedResource, moduleParams, moduleResources, logicalId, templateProps)
 		outputNode.Content = append(outputNode.Content, clonedResource)
 	}
 
@@ -1052,8 +928,6 @@ func module(ctx *directiveContext) (bool, error) {
 		}
 	}
 
-	// TODO: Download if it's a zip
-
 	// Parse the file
 	var moduleNode yaml.Node
 	err = yaml.Unmarshal(content, &moduleNode)
@@ -1077,7 +951,15 @@ func module(ctx *directiveContext) (bool, error) {
 	moduleAsTemplate := cft.Template{Node: &moduleNode}
 
 	// Read things like Constants
+	config.Debugf("About to processRainSection in %s", uri)
 	processRainSection(&moduleAsTemplate)
+	for k, v := range moduleAsTemplate.Constants {
+		config.Debugf("%s = %s", k, v.Value)
+	}
+
+	if moduleAsTemplate.Constants != nil {
+		replaceTemplateConstants(moduleAsTemplate.Node, moduleAsTemplate.Constants)
+	}
 
 	_, err = transform(&transformContext{
 		nodeToTransform: &moduleNode,
@@ -1093,8 +975,9 @@ func module(ctx *directiveContext) (bool, error) {
 
 	// Create a new node to represent the processed module
 	var outputNode yaml.Node
-	_, err = processModule(&moduleNode, &outputNode, t, n, parent)
+	_, err = processModule(&moduleNode, &outputNode, t, n, parent, moduleAsTemplate.Constants)
 	if err != nil {
+		config.Debugf("processModule error: %v, moduleNode: %s", err, node.ToSJson(&moduleNode))
 		return false, fmt.Errorf("failed to process module %s: %v", uri, err)
 	}
 
diff --git a/cft/pkg/module_test.go b/cft/pkg/module_test.go
index 68aa7d7d..70ecf82a 100644
--- a/cft/pkg/module_test.go
+++ b/cft/pkg/module_test.go
@@ -7,7 +7,6 @@ import (
 	"github.com/aws-cloudformation/rain/cft/diff"
 	"github.com/aws-cloudformation/rain/cft/parse"
 	"github.com/aws-cloudformation/rain/cft/pkg"
-	"github.com/aws-cloudformation/rain/internal/node"
 	"gopkg.in/yaml.v3"
 )
 
@@ -63,6 +62,10 @@ func TestIfPAram(t *testing.T) {
 	runTest("ifparam", t)
 }
 
+func TestConstant(t *testing.T) {
+	runTest("constant", t)
+}
+
 // TODO: This was broken in the refactor, come back to it later
 //func TestForeach(t *testing.T) {
 //	runTest("foreach", t)
@@ -125,36 +128,3 @@ func TestCsvToSequence(t *testing.T) {
 		t.Errorf("Unexpected sequence")
 	}
 }
-
-func TestMergeNodes(t *testing.T) {
-	original := &yaml.Node{Kind: yaml.MappingNode, Content: make([]*yaml.Node, 0)}
-	override := &yaml.Node{Kind: yaml.MappingNode, Content: make([]*yaml.Node, 0)}
-	expected := &yaml.Node{Kind: yaml.MappingNode, Content: make([]*yaml.Node, 0)}
-
-	original.Content = append(original.Content,
-		&yaml.Node{Kind: yaml.ScalarNode, Value: "A"})
-	original.Content = append(original.Content,
-		&yaml.Node{Kind: yaml.ScalarNode, Value: "foo"})
-
-	override.Content = append(override.Content,
-		&yaml.Node{Kind: yaml.ScalarNode, Value: "A"})
-	override.Content = append(override.Content,
-		&yaml.Node{Kind: yaml.ScalarNode, Value: "bar"})
-
-	expected.Content = append(expected.Content,
-		&yaml.Node{Kind: yaml.ScalarNode, Value: "A"})
-	expected.Content = append(expected.Content,
-		&yaml.Node{Kind: yaml.ScalarNode, Value: "bar"})
-
-	merged := pkg.MergeNodes(original, override)
-
-	diff := node.Diff(merged, expected)
-
-	if len(diff) > 0 {
-		for _, d := range diff {
-			fmt.Println(d)
-		}
-		t.Fatalf("nodes are not the same")
-	}
-
-}
diff --git a/cft/pkg/tmpl/constant-expect.yaml b/cft/pkg/tmpl/constant-expect.yaml
new file mode 100644
index 00000000..568ab0f0
--- /dev/null
+++ b/cft/pkg/tmpl/constant-expect.yaml
@@ -0,0 +1,6 @@
+Resources:
+  BucketBucket:
+    Type: AWS::S3::Bucket
+    Properties:
+      BucketName: bar
+
diff --git a/cft/pkg/tmpl/constant-module.yaml b/cft/pkg/tmpl/constant-module.yaml
new file mode 100644
index 00000000..a9210839
--- /dev/null
+++ b/cft/pkg/tmpl/constant-module.yaml
@@ -0,0 +1,10 @@
+Rain:
+  Constants:
+    foo: bar
+
+Resources:
+  Bucket:
+    Type: AWS::S3::Bucket
+    Properties:
+      BucketName: !Sub ${Rain::foo}
+
diff --git a/cft/pkg/tmpl/constant-template.yaml b/cft/pkg/tmpl/constant-template.yaml
new file mode 100644
index 00000000..631cb6ec
--- /dev/null
+++ b/cft/pkg/tmpl/constant-template.yaml
@@ -0,0 +1,4 @@
+Resources:
+  Bucket:
+    Type: !Rain::Module ./constant-module.yaml
+
diff --git a/cft/pkg/tmpl/ifparam-expect.yaml b/cft/pkg/tmpl/ifparam-expect.yaml
index f6f17b36..39b4e88d 100644
--- a/cft/pkg/tmpl/ifparam-expect.yaml
+++ b/cft/pkg/tmpl/ifparam-expect.yaml
@@ -1,6 +1,4 @@
 Resources:
   ShowBucket:
     Type: AWS::S3::Bucket
-    Metadata:
-      Rain: {}
 
diff --git a/internal/node/node.go b/internal/node/node.go
index c4657121..576dab5d 100644
--- a/internal/node/node.go
+++ b/internal/node/node.go
@@ -347,3 +347,92 @@ func appendDiffs(diffs []string, node1, node2 *yaml.Node) []string {
 	}
 	return append(diffs, Diff(node1, node2)...)
 }
+
+// MergeNodes merges two mapping nodes, replacing any values found in override
+func MergeNodes(original *yaml.Node, override *yaml.Node) *yaml.Node {
+
+	// If the nodes are not the same kind, just return the override
+	if override.Kind != original.Kind {
+		return override
+	}
+
+	if override.Kind == yaml.ScalarNode {
+		return override
+	}
+
+	retval := &yaml.Node{Kind: override.Kind, Content: make([]*yaml.Node, 0)}
+	overrideMap := make(map[string]bool)
+
+	if override.Kind == yaml.SequenceNode {
+		retval.Content = append(retval.Content, override.Content...)
+
+		for _, n := range original.Content {
+			already := false
+			for _, r := range retval.Content {
+				if r.Value == n.Value {
+					already = true
+					break
+				}
+			}
+			if !already {
+				retval.Content = append(retval.Content, n)
+			}
+		}
+
+		return retval
+	}
+	// TODO: Not working for adding statements to a Policy
+
+	// else they are both Mapping nodes
+
+	// Add everything in the override Mapping
+	for i, n := range override.Content {
+		retval.Content = append(retval.Content, n)
+		var name string
+		if i%2 == 0 {
+			// Remember what we added
+			name = n.Value
+			overrideMap[name] = true
+		} else {
+			name = retval.Content[i-1].Value
+		}
+
+		/*
+			Original:
+				A: something
+				B:
+					foo: 1
+					bar: 2
+
+			Override:
+				A: something else
+				B:
+					foo: 3
+					baz: 6
+		*/
+
+		// Recurse if this is a mapping node
+		if i%2 == 1 && n.Kind == yaml.MappingNode {
+			// Find the matching node in original
+			for j, match := range original.Content {
+				if j%2 == 0 && match.Value == name {
+					n.Content[i] = MergeNodes(n.Content[i], original.Content[j])
+				}
+			}
+		}
+	}
+
+	// Only add things from the original if they weren't in original
+	for i := 0; i < len(original.Content); i++ {
+		n := original.Content[i]
+		if i%2 == 0 {
+			if _, exists := overrideMap[n.Value]; exists {
+				i = i + 1 // Skip the next node
+				continue
+			}
+		}
+		retval.Content = append(retval.Content, n)
+	}
+
+	return retval
+}
diff --git a/internal/node/node_test.go b/internal/node/node_test.go
index 99291389..a7d00ac9 100644
--- a/internal/node/node_test.go
+++ b/internal/node/node_test.go
@@ -1,6 +1,7 @@
 package node_test
 
 import (
+	"fmt"
 	"testing"
 
 	"github.com/aws-cloudformation/rain/internal/node"
@@ -205,3 +206,36 @@ func TestDiff(t *testing.T) {
 	}
 
 }
+
+func TestMergeNodes(t *testing.T) {
+	original := &yaml.Node{Kind: yaml.MappingNode, Content: make([]*yaml.Node, 0)}
+	override := &yaml.Node{Kind: yaml.MappingNode, Content: make([]*yaml.Node, 0)}
+	expected := &yaml.Node{Kind: yaml.MappingNode, Content: make([]*yaml.Node, 0)}
+
+	original.Content = append(original.Content,
+		&yaml.Node{Kind: yaml.ScalarNode, Value: "A"})
+	original.Content = append(original.Content,
+		&yaml.Node{Kind: yaml.ScalarNode, Value: "foo"})
+
+	override.Content = append(override.Content,
+		&yaml.Node{Kind: yaml.ScalarNode, Value: "A"})
+	override.Content = append(override.Content,
+		&yaml.Node{Kind: yaml.ScalarNode, Value: "bar"})
+
+	expected.Content = append(expected.Content,
+		&yaml.Node{Kind: yaml.ScalarNode, Value: "A"})
+	expected.Content = append(expected.Content,
+		&yaml.Node{Kind: yaml.ScalarNode, Value: "bar"})
+
+	merged := node.MergeNodes(original, override)
+
+	diff := node.Diff(merged, expected)
+
+	if len(diff) > 0 {
+		for _, d := range diff {
+			fmt.Println(d)
+		}
+		t.Fatalf("nodes are not the same")
+	}
+
+}
diff --git a/modules/compliant-bucket.yaml b/modules/compliant-bucket.yaml
index 566e0255..e9f01237 100644
--- a/modules/compliant-bucket.yaml
+++ b/modules/compliant-bucket.yaml
@@ -22,11 +22,11 @@ Parameters:
 
 Rain:
   Constants:
-    S3Arn: arn:${AWS::Partition}:s3:::
-    BucketName: ${AppName}-${AWS::Region}-${AWS::AccountId}
-    LogBucketName: ${AppName}-logs-${AWS::Region}-${AWS::AccountId}
-    LogBucketArn: ${Rain::S3Arn}${Rain::LogBucketName}
-    ReplicaBucketName: ${AppName}-replicas-${AWS::Region}-${AWS::AccountId}
+    S3Arn: "arn:${AWS::Partition}:s3:::"
+    BucketName: "${AppName}-${AWS::Region}-${AWS::AccountId}"
+    LogBucketName: "${AppName}-logs-${AWS::Region}-${AWS::AccountId}"
+    LogBucketArn: "${Rain::S3Arn}${Rain::LogBucketName}"
+    ReplicaBucketName: "${AppName}-replicas-${AWS::Region}-${AWS::AccountId}"
 
 
 Resources:

From cc3c275983bb1304154377a0ec402940116ea52e Mon Sep 17 00:00:00 2001
From: Eric Beard <ericzbeard@gmail.com>
Date: Thu, 7 Nov 2024 17:27:22 -0800
Subject: [PATCH 06/15] Webapp deploys again

---
 cft/pkg/module.go                  |  35 ++--
 cft/pkg/tmpl/ifparam-expect.yaml   |   2 +
 cft/pkg/tmpl/ifparam-template.yaml |   4 +
 cft/pkg/tmpl/sub-module.yaml       |   2 +-
 modules/compliant-bucket.yaml      |   4 +-
 modules/simple-table.yaml          |   6 +-
 test/webapp/webapp-pkg.yaml        | 282 ++++++++++++-----------------
 test/webapp/webapp.yaml            |   4 +-
 8 files changed, 152 insertions(+), 187 deletions(-)

diff --git a/cft/pkg/module.go b/cft/pkg/module.go
index 74a447f5..f041fad0 100644
--- a/cft/pkg/module.go
+++ b/cft/pkg/module.go
@@ -378,22 +378,22 @@ func resolveModuleSub(parentName string, prop *yaml.Node, sidx int, ctx *refctx)
 			}
 			sub += fmt.Sprintf("${%s.%s}", left, right)
 			needSub = true
-		case parse.RAIN:
-			// Replace ${Rain::ConstantName} with template constant value
-			if ctx.constants == nil {
-				return fmt.Errorf("no Rain Constants section, looking for %s", word.W)
-			}
-			if c, ok := ctx.constants[word.W]; ok {
-				sub += c.Value
-			} else {
-				if len(ctx.constants) == 0 {
-					config.Debugf("Constants are empty")
-				}
-				for k, v := range ctx.constants {
-					config.Debugf("Constant %s: %s", k, v.Value)
-				}
-				return fmt.Errorf("unable to find Rain constant %s", word.W)
-			}
+		//case parse.RAIN:
+		//	// Replace ${Rain::ConstantName} with template constant value
+		//	if ctx.constants == nil {
+		//		return fmt.Errorf("no Rain Constants section, looking for %s", word.W)
+		//	}
+		//	if c, ok := ctx.constants[word.W]; ok {
+		//		sub += c.Value
+		//	} else {
+		//		if len(ctx.constants) == 0 {
+		//			config.Debugf("Constants are empty")
+		//		}
+		//		for k, v := range ctx.constants {
+		//			config.Debugf("Constant %s: %s", k, v.Value)
+		//		}
+		//		return fmt.Errorf("unable to find Rain constant %s", word.W)
+		//	}
 		default:
 			return fmt.Errorf("unexpected word type %v for %s", word.T, word.W)
 		}
@@ -637,7 +637,8 @@ func processModule(
 				if ifp != "" {
 					if moduleParams != nil &&
 						s11n.GetMap(moduleParams, ifp) != nil &&
-						s11n.GetValue(templateProps, ifp) == "" {
+						s11n.GetValue(templateProps, ifp) == "" &&
+						len(s11n.GetMap(templateProps, ifp)) == 0 {
 						continue
 					} else {
 						// Get rid of the IfParam, since it's irrelevant in the resulting template
diff --git a/cft/pkg/tmpl/ifparam-expect.yaml b/cft/pkg/tmpl/ifparam-expect.yaml
index 39b4e88d..d63b1fe1 100644
--- a/cft/pkg/tmpl/ifparam-expect.yaml
+++ b/cft/pkg/tmpl/ifparam-expect.yaml
@@ -1,4 +1,6 @@
 Resources:
   ShowBucket:
     Type: AWS::S3::Bucket
+  Show2Bucket:
+    Type: AWS::S3::Bucket
 
diff --git a/cft/pkg/tmpl/ifparam-template.yaml b/cft/pkg/tmpl/ifparam-template.yaml
index 371b1f1f..4b97a601 100644
--- a/cft/pkg/tmpl/ifparam-template.yaml
+++ b/cft/pkg/tmpl/ifparam-template.yaml
@@ -3,5 +3,9 @@ Resources:
     Type: !Rain::Module ./ifparam-module.yaml
     Properties:
       Show: true
+  Show2:
+    Type: !Rain::Module ./ifparam-module.yaml
+    Properties:
+      Show: !Sub ${Foo}
   Hide:
     Type: !Rain::Module ./ifparam-module.yaml
diff --git a/cft/pkg/tmpl/sub-module.yaml b/cft/pkg/tmpl/sub-module.yaml
index b088c597..8d4937f4 100644
--- a/cft/pkg/tmpl/sub-module.yaml
+++ b/cft/pkg/tmpl/sub-module.yaml
@@ -7,7 +7,7 @@ Resources:
   Bucket1:
     Type: AWS::S3::Bucket
     Properties:
-      BucketName: !Sub "${Name}"
+      BucketName: !Sub ${Name}
       X: !Sub "${SubName}"
       Y:
         - !Sub noparent0-${SubName}
diff --git a/modules/compliant-bucket.yaml b/modules/compliant-bucket.yaml
index e9f01237..290e8d3e 100644
--- a/modules/compliant-bucket.yaml
+++ b/modules/compliant-bucket.yaml
@@ -89,14 +89,14 @@ Resources:
               - Action: s3:PutObject
                 Condition:
                   ArnLike:
-                    aws:SourceArn: ${Rain::LogBucketArn}/*
+                    aws:SourceArn: !Sub ${Rain::LogBucketArn}/*
                   StringEquals:
                     aws:SourceAccount: !Ref AWS::AccountId
                 Effect: Allow
                 Principal:
                   Service: logging.s3.amazonaws.com
                 Resource:
-                  - !Sub ${Rain::LogBucketName}/*
+                  - !Sub ${Rain::LogBucketArn}/*
 
   Bucket:
     Type: AWS::S3::Bucket
diff --git a/modules/simple-table.yaml b/modules/simple-table.yaml
index 089dc15a..ab7dcf0c 100644
--- a/modules/simple-table.yaml
+++ b/modules/simple-table.yaml
@@ -1,9 +1,9 @@
-Properties:
+Parameters:
 
   TableName:
     Type: String
   
-  LambdaRoleArn:
+  LambdaRole:
     Type: String
     Description: If set, allow the lambda function to access this table
 
@@ -41,5 +41,5 @@ Resources:
             Resource: 
               - !GetAtt Table.Arn
       PolicyName: !Sub ${TableName}-policy
-      RoleName: !Ref LambdaRoleArn
+      RoleName: !Ref LambdaRole
 
diff --git a/test/webapp/webapp-pkg.yaml b/test/webapp/webapp-pkg.yaml
index 0d361e25..ffdca8ac 100644
--- a/test/webapp/webapp-pkg.yaml
+++ b/test/webapp/webapp-pkg.yaml
@@ -2,42 +2,11 @@ Description: "Creates a web application with a static website using S3 and Cloud
 
 Parameters:
   AppName:
-    Type: String
     Description: This name is used as a prefix for resource names
+    Type: String
     Default: rain-webapp
 
 Resources:
-  TestResourceHandlerPolicy:
-    Type: AWS::IAM::RolePolicy
-    Properties:
-      PolicyDocument:
-        Statement:
-          - Action:
-              - dynamodb:BatchGetItem
-              - dynamodb:GetItem
-              - dynamodb:Query
-              - dynamodb:Scan
-              - dynamodb:BatchWriteItem
-              - dynamodb:PutItem
-              - dynamodb:UpdateItem
-            Effect: Allow
-            Resource:
-              - !GetAtt TestTable.Arn
-      PolicyName: handler-policy
-      RoleName: !Ref TestResourceHandlerRole
-
-  TestTable:
-    Type: AWS::DynamoDB::Table
-    Properties:
-      BillingMode: PAY_PER_REQUEST
-      TableName: !Sub ${AppName}-test
-      AttributeDefinitions:
-        - AttributeName: id
-          AttributeType: S
-      KeySchema:
-        - AttributeName: id
-          KeyType: HASH
-
   SiteOriginAccessControl:
     Type: AWS::CloudFront::OriginAccessControl
     Properties:
@@ -56,6 +25,16 @@ Resources:
 
   SiteDistribution:
     Type: AWS::CloudFront::Distribution
+    Metadata:
+      checkov:
+        skip:
+          - id: CKV_AWS_174
+            comment: Using the default cloudfront certificate with no aliases
+      guard:
+        SuppressedRules:
+          - CLOUDFRONT_CUSTOM_SSL_CERTIFICATE
+          - CLOUDFRONT_ORIGIN_FAILOVER_ENABLED
+          - CLOUDFRONT_SNI_ENABLED
     Properties:
       DistributionConfig:
         DefaultCacheBehavior:
@@ -78,16 +57,6 @@ Resources:
         ViewerCertificate:
           CloudFrontDefaultCertificate: true
         WebACLId: !GetAtt SiteWebACL.Arn
-    Metadata:
-      checkov:
-        skip:
-          - id: CKV_AWS_174
-            comment: Using the default cloudfront certificate with no aliases
-      guard:
-        SuppressedRules:
-          - CLOUDFRONT_CUSTOM_SSL_CERTIFICATE
-          - CLOUDFRONT_ORIGIN_FAILOVER_ENABLED
-          - CLOUDFRONT_SNI_ENABLED
 
   SiteWebACL:
     Type: AWS::WAFv2::WebACL
@@ -122,26 +91,6 @@ Resources:
 
   SiteContentLogBucket:
     Type: AWS::S3::Bucket
-    Properties:
-      BucketEncryption:
-        ServerSideEncryptionConfiguration:
-          - ServerSideEncryptionByDefault:
-              SSEAlgorithm: AES256
-      BucketName: !Sub ${AppName}-content-logs-${AWS::Region}-${AWS::AccountId}
-      ObjectLockConfiguration:
-        ObjectLockEnabled: Enabled
-        Rule:
-          DefaultRetention:
-            Mode: COMPLIANCE
-            Years: 1
-      ObjectLockEnabled: true
-      PublicAccessBlockConfiguration:
-        BlockPublicAcls: true
-        BlockPublicPolicy: true
-        IgnorePublicAcls: true
-        RestrictPublicBuckets: true
-      VersioningConfiguration:
-        Status: Enabled
     Metadata:
       Comment: This bucket records access logs for the main bucket
       checkov:
@@ -155,31 +104,29 @@ Resources:
       Rain:
         Content: RAIN_NO_CONTENT
         EmptyOnDelete: true
-
-  SiteContentBucket:
-    Type: AWS::S3::Bucket
     Properties:
       BucketEncryption:
         ServerSideEncryptionConfiguration:
           - ServerSideEncryptionByDefault:
               SSEAlgorithm: AES256
-      BucketName: !Sub ${AppName}-content-${AWS::Region}-${AWS::AccountId}
-      LoggingConfiguration:
-        DestinationBucketName: !Ref SiteContentLogBucket
-      ObjectLockEnabled: false
+      BucketName: !Sub ${AppName}-content-logs-${AWS::Region}-${AWS::AccountId}
+      ObjectLockConfiguration:
+        ObjectLockEnabled: Enabled
+        Rule:
+          DefaultRetention:
+            Mode: COMPLIANCE
+            Years: 1
+      ObjectLockEnabled: true
       PublicAccessBlockConfiguration:
         BlockPublicAcls: true
         BlockPublicPolicy: true
         IgnorePublicAcls: true
         RestrictPublicBuckets: true
-      ReplicationConfiguration:
-        Role: !GetAtt SiteContentReplicationRole.Arn
-        Rules:
-          - Destination:
-              Bucket: !GetAtt SiteContentReplicaBucket.Arn
-            Status: Enabled
       VersioningConfiguration:
         Status: Enabled
+
+  SiteContentBucket:
+    Type: AWS::S3::Bucket
     Metadata:
       guard:
         SuppressedRules:
@@ -198,23 +145,31 @@ Resources:
             - Rain::OutputValue RedirectURI
             - Rain::OutputValue AppName
             - Rain::OutputValue AppClientId
-
-  SiteContentReplicaBucket:
-    Type: AWS::S3::Bucket
     Properties:
       BucketEncryption:
         ServerSideEncryptionConfiguration:
           - ServerSideEncryptionByDefault:
               SSEAlgorithm: AES256
-      BucketName: !Sub ${AppName}-content-replicas-${AWS::Region}-${AWS::AccountId}
+      BucketName: !Sub ${AppName}-content-${AWS::Region}-${AWS::AccountId}
+      LoggingConfiguration:
+        DestinationBucketName: !Ref SiteContentLogBucket
       ObjectLockEnabled: false
       PublicAccessBlockConfiguration:
         BlockPublicAcls: true
         BlockPublicPolicy: true
         IgnorePublicAcls: true
         RestrictPublicBuckets: true
+      ReplicationConfiguration:
+        Role: !GetAtt SiteContentReplicationRole.Arn
+        Rules:
+          - Destination:
+              Bucket: !GetAtt SiteContentReplicaBucket.Arn
+            Status: Enabled
       VersioningConfiguration:
         Status: Enabled
+
+  SiteContentReplicaBucket:
+    Type: AWS::S3::Bucket
     Metadata:
       Comment: This bucket is used as a target for replicas from the main bucket
       checkov:
@@ -229,6 +184,20 @@ Resources:
       Rain:
         Content: RAIN_NO_CONTENT
         EmptyOnDelete: true
+    Properties:
+      BucketEncryption:
+        ServerSideEncryptionConfiguration:
+          - ServerSideEncryptionByDefault:
+              SSEAlgorithm: AES256
+      BucketName: !Sub ${AppName}-content-replicas-${AWS::Region}-${AWS::AccountId}
+      ObjectLockEnabled: false
+      PublicAccessBlockConfiguration:
+        BlockPublicAcls: true
+        BlockPublicPolicy: true
+        IgnorePublicAcls: true
+        RestrictPublicBuckets: true
+      VersioningConfiguration:
+        Status: Enabled
 
   SiteContentReplicationPolicy:
     Type: AWS::IAM::RolePolicy
@@ -289,7 +258,7 @@ Resources:
           - Action: s3:PutObject
             Condition:
               ArnLike:
-                aws:SourceArn: !Sub arn:${AWS::Partition}:s3:::${AppName}-content-logs-${AWS::Region}-${AWS::AccountId}
+                aws:SourceArn: !Sub arn:${AWS::Partition}:s3:::${AppName}-content-logs-${AWS::Region}-${AWS::AccountId}/*
               StringEquals:
                 aws:SourceAccount: !Ref AWS::AccountId
             Effect: Allow
@@ -323,17 +292,6 @@ Resources:
             Resource:
               - !Sub arn:${AWS::Partition}:s3:::${AppName}-content-${AWS::Region}-${AWS::AccountId}
               - !Sub arn:${AWS::Partition}:s3:::${AppName}-content-${AWS::Region}-${AWS::AccountId}/*
-          - Action: s3:PutObject
-            Condition:
-              ArnLike:
-                aws:SourceArn: !Sub arn:${AWS::Partition}:s3:::${AppName}-content-${AWS::Region}-${AWS::AccountId}
-              StringEquals:
-                aws:SourceAccount: !Ref AWS::AccountId
-            Effect: Allow
-            Principal:
-              Service: logging.s3.amazonaws.com
-            Resource:
-              - !Sub arn:${AWS::Partition}:s3:::${AppName}-content-${AWS::Region}-${AWS::AccountId}/*
         Version: "2012-10-17"
 
   SiteContentReplicaBucketAccessPolicy:
@@ -352,21 +310,23 @@ Resources:
             Resource:
               - !Sub arn:${AWS::Partition}:s3:::${AppName}-content-replicas-${AWS::Region}-${AWS::AccountId}
               - !Sub arn:${AWS::Partition}:s3:::${AppName}-content-replicas-${AWS::Region}-${AWS::AccountId}/*
-          - Action: s3:PutObject
-            Condition:
-              ArnLike:
-                aws:SourceArn: !Sub arn:${AWS::Partition}:s3:::${AppName}-content-replicas-${AWS::Region}-${AWS::AccountId}
-              StringEquals:
-                aws:SourceAccount: !Ref AWS::AccountId
-            Effect: Allow
-            Principal:
-              Service: logging.s3.amazonaws.com
-            Resource:
-              - !Sub arn:${AWS::Partition}:s3:::${AppName}-content-replicas-${AWS::Region}-${AWS::AccountId}/*
         Version: "2012-10-17"
 
   SiteCloudFrontLogsLogBucket:
     Type: AWS::S3::Bucket
+    Metadata:
+      Comment: This bucket records access logs for the main bucket
+      checkov:
+        skip:
+          - comment: This is the log bucket
+            id: CKV_AWS_18
+      guard:
+        SuppressedRules:
+          - S3_BUCKET_LOGGING_ENABLED
+          - S3_BUCKET_REPLICATION_ENABLED
+      Rain:
+        Content: RAIN_NO_CONTENT
+        EmptyOnDelete: true
     Properties:
       BucketEncryption:
         ServerSideEncryptionConfiguration:
@@ -387,22 +347,17 @@ Resources:
         RestrictPublicBuckets: true
       VersioningConfiguration:
         Status: Enabled
+
+  SiteCloudFrontLogsBucket:
+    Type: AWS::S3::Bucket
     Metadata:
-      Comment: This bucket records access logs for the main bucket
-      checkov:
-        skip:
-          - comment: This is the log bucket
-            id: CKV_AWS_18
       guard:
         SuppressedRules:
-          - S3_BUCKET_LOGGING_ENABLED
-          - S3_BUCKET_REPLICATION_ENABLED
+          - S3_BUCKET_DEFAULT_LOCK_ENABLED
       Rain:
         Content: RAIN_NO_CONTENT
         EmptyOnDelete: true
-
-  SiteCloudFrontLogsBucket:
-    Type: AWS::S3::Bucket
+        DistributionLogicalId: NONE
     Properties:
       BucketEncryption:
         ServerSideEncryptionConfiguration:
@@ -428,17 +383,23 @@ Resources:
       OwnershipControls:
         Rules:
           - ObjectOwnership: BucketOwnerPreferred
+
+  SiteCloudFrontLogsReplicaBucket:
+    Type: AWS::S3::Bucket
     Metadata:
+      Comment: This bucket is used as a target for replicas from the main bucket
+      checkov:
+        skip:
+          - comment: This is the replica bucket
+            id: CKV_AWS_18
       guard:
         SuppressedRules:
           - S3_BUCKET_DEFAULT_LOCK_ENABLED
+          - S3_BUCKET_REPLICATION_ENABLED
+          - S3_BUCKET_LOGGING_ENABLED
       Rain:
         Content: RAIN_NO_CONTENT
         EmptyOnDelete: true
-        DistributionLogicalId: NONE
-
-  SiteCloudFrontLogsReplicaBucket:
-    Type: AWS::S3::Bucket
     Properties:
       BucketEncryption:
         ServerSideEncryptionConfiguration:
@@ -453,20 +414,6 @@ Resources:
         RestrictPublicBuckets: true
       VersioningConfiguration:
         Status: Enabled
-    Metadata:
-      Comment: This bucket is used as a target for replicas from the main bucket
-      checkov:
-        skip:
-          - comment: This is the replica bucket
-            id: CKV_AWS_18
-      guard:
-        SuppressedRules:
-          - S3_BUCKET_DEFAULT_LOCK_ENABLED
-          - S3_BUCKET_REPLICATION_ENABLED
-          - S3_BUCKET_LOGGING_ENABLED
-      Rain:
-        Content: RAIN_NO_CONTENT
-        EmptyOnDelete: true
 
   SiteCloudFrontLogsReplicationPolicy:
     Type: AWS::IAM::RolePolicy
@@ -527,7 +474,7 @@ Resources:
           - Action: s3:PutObject
             Condition:
               ArnLike:
-                aws:SourceArn: !Sub arn:${AWS::Partition}:s3:::${AppName}-cflogs-logs-${AWS::Region}-${AWS::AccountId}
+                aws:SourceArn: !Sub arn:${AWS::Partition}:s3:::${AppName}-cflogs-logs-${AWS::Region}-${AWS::AccountId}/*
               StringEquals:
                 aws:SourceAccount: !Ref AWS::AccountId
             Effect: Allow
@@ -553,17 +500,6 @@ Resources:
             Resource:
               - !Sub arn:${AWS::Partition}:s3:::${AppName}-cflogs-${AWS::Region}-${AWS::AccountId}
               - !Sub arn:${AWS::Partition}:s3:::${AppName}-cflogs-${AWS::Region}-${AWS::AccountId}/*
-          - Action: s3:PutObject
-            Condition:
-              ArnLike:
-                aws:SourceArn: !Sub arn:${AWS::Partition}:s3:::${AppName}-cflogs-${AWS::Region}-${AWS::AccountId}
-              StringEquals:
-                aws:SourceAccount: !Ref AWS::AccountId
-            Effect: Allow
-            Principal:
-              Service: logging.s3.amazonaws.com
-            Resource:
-              - !Sub arn:${AWS::Partition}:s3:::${AppName}-cflogs-${AWS::Region}-${AWS::AccountId}/*
         Version: "2012-10-17"
 
   SiteCloudFrontLogsReplicaBucketAccessPolicy:
@@ -582,21 +518,12 @@ Resources:
             Resource:
               - !Sub arn:${AWS::Partition}:s3:::${AppName}-cflogs-replicas-${AWS::Region}-${AWS::AccountId}
               - !Sub arn:${AWS::Partition}:s3:::${AppName}-cflogs-replicas-${AWS::Region}-${AWS::AccountId}/*
-          - Action: s3:PutObject
-            Condition:
-              ArnLike:
-                aws:SourceArn: !Sub arn:${AWS::Partition}:s3:::${AppName}-cflogs-replicas-${AWS::Region}-${AWS::AccountId}
-              StringEquals:
-                aws:SourceAccount: !Ref AWS::AccountId
-            Effect: Allow
-            Principal:
-              Service: logging.s3.amazonaws.com
-            Resource:
-              - !Sub arn:${AWS::Partition}:s3:::${AppName}-cflogs-replicas-${AWS::Region}-${AWS::AccountId}/*
         Version: "2012-10-17"
 
   CognitoUserPool:
     Type: AWS::Cognito::UserPool
+    DependsOn:
+      - SiteDistribution
     Properties:
       UserPoolName: !Ref AppName
       AdminCreateUserConfig:
@@ -610,8 +537,6 @@ Resources:
           Required: true
         - Name: family_name
           Required: true
-    DependsOn:
-      - SiteDistribution
 
   CognitoDomain:
     Type: AWS::Cognito::UserPoolDomain
@@ -645,11 +570,11 @@ Resources:
       Runtime: provided.al2023
       Code:
         S3Bucket: rain-artifacts-207567786752-us-east-1
-        S3Key: db3706b9e9ec0046b308635fbc9ecdb3a4ad31e1069210e881e5442f546cc285
+        S3Key: 66657f52f75173dd78bf54255eec803c9eb4fd2485e702c6724200bacad6770a
       Role: !GetAtt TestResourceHandlerRole.Arn
       Environment:
         Variables:
-          TABLE_NAME: !Ref TestTable
+          TABLE_NAME: !Ref TestDataTable
 
   TestResourceHandlerRole:
     Type: AWS::IAM::Role
@@ -722,7 +647,7 @@ Resources:
       Runtime: provided.al2023
       Code:
         S3Bucket: rain-artifacts-207567786752-us-east-1
-        S3Key: 7b300d8fa211e93b14974d3e699a8c479470a1b31a6202176441cf11e5ad93f3
+        S3Key: 175e360cbc251193e4e3113b9fe7aecfbe6af368cf60447152b558b0a1d554c8
       Role: !GetAtt JwtResourceHandlerRole.Arn
       Environment:
         Variables:
@@ -802,15 +727,15 @@ Resources:
 
   RestApiDeployment:
     Type: AWS::ApiGateway::Deployment
-    Properties:
-      RestApiId: !Ref RestApi
-    Metadata:
-      Version: 2
     DependsOn:
       - TestResourceGet
       - TestResourceOptions
       - JwtResourceGet
       - JwtResourceOptions
+    Metadata:
+      Version: 2
+    Properties:
+      RestApiId: !Ref RestApi
 
   RestApiStage:
     Type: AWS::ApiGateway::Stage
@@ -829,6 +754,39 @@ Resources:
       RestApiId: !Ref RestApi
       Type: COGNITO_USER_POOLS
 
+  TestDataTable:
+    Type: AWS::DynamoDB::Table
+    Properties:
+      BillingMode: PAY_PER_REQUEST
+      TableName: !Sub ${AppName}-test
+      AttributeDefinitions:
+        - AttributeName: id
+          AttributeType: S
+      KeySchema:
+        - AttributeName: id
+          KeyType: HASH
+
+  TestDataLambdaPolicy:
+    Type: AWS::IAM::RolePolicy
+    Metadata:
+      Comment: This resource is created only if the LambdaRoleArn is set
+    Properties:
+      PolicyDocument:
+        Statement:
+          - Action:
+              - dynamodb:BatchGetItem
+              - dynamodb:GetItem
+              - dynamodb:Query
+              - dynamodb:Scan
+              - dynamodb:BatchWriteItem
+              - dynamodb:PutItem
+              - dynamodb:UpdateItem
+            Effect: Allow
+            Resource:
+              - !GetAtt TestDataTable.Arn
+      PolicyName: !Sub ${AppName}-test-policy
+      RoleName: !Ref TestResourceHandlerRole
+
 Outputs:
   SiteURL:
     Value: !Sub https://${SiteDistribution.DomainName}
diff --git a/test/webapp/webapp.yaml b/test/webapp/webapp.yaml
index 1ab155b5..7a8a5161 100644
--- a/test/webapp/webapp.yaml
+++ b/test/webapp/webapp.yaml
@@ -82,13 +82,13 @@ Resources:
         Properties:
           Environment:
             Variables:
-              TABLE_NAME: !Ref TestTable
+              TABLE_NAME: !Ref TestDataTable
 
   TestData:
     Type: !Rain::Module "../../modules/simple-table.yaml" 
     Properties:
       TableName: !Sub ${AppName}-test
-      LambdaRoleArn: !GetAtt TestResourceHandlerRole.Arn 
+      LambdaRole: !Ref TestResourceHandlerRole
 
   JwtResource:
     Type: !Rain::Module "../../modules/api-resource.yaml"

From 43478604bb2b6504f6db2323e85d2e3911363b35 Mon Sep 17 00:00:00 2001
From: Eric Beard <ericzbeard@gmail.com>
Date: Fri, 8 Nov 2024 09:12:25 -0800
Subject: [PATCH 07/15] Webapp is functional again

---
 cft/cft.go                     | 3 ++-
 internal/cmd/deploy/content.go | 8 --------
 test/webapp/site/js/app.js     | 8 ++++----
 test/webapp/webapp-pkg.yaml    | 2 +-
 4 files changed, 7 insertions(+), 14 deletions(-)

diff --git a/cft/cft.go b/cft/cft.go
index b200a3a7..0698f1ef 100644
--- a/cft/cft.go
+++ b/cft/cft.go
@@ -141,7 +141,8 @@ func (t Template) GetSection(section Section) (*yaml.Node, error) {
 	if t.Node == nil {
 		return nil, fmt.Errorf("unable to get section because t.Node is nil")
 	}
-	_, s, _ := s11n.GetMapValue(t.Node.Content[0], string(section))
+	m := t.Node.Content[0]
+	_, s, _ := s11n.GetMapValue(m, string(section))
 	if s == nil {
 		config.Debugf("GetSection t.Node: %s", node.ToSJson(t.Node))
 		return nil, fmt.Errorf("unable to locate the %s node", section)
diff --git a/internal/cmd/deploy/content.go b/internal/cmd/deploy/content.go
index ac8cf28b..bd63f1d6 100644
--- a/internal/cmd/deploy/content.go
+++ b/internal/cmd/deploy/content.go
@@ -70,10 +70,6 @@ func addCommandArgs(run *yaml.Node, cmd *exec.Cmd, isBefore bool, stackName stri
 // are any errors. Then after deployment, the Content node is processed
 func processMetadataBefore(template cft.Template, stackName string, rootDir string) error {
 
-	// For some reason Package created an extra document node
-	// (And CreateChangeSet is ok with this...?)
-	template.Node = template.Node.Content[0]
-
 	buckets := template.GetResourcesOfType("AWS::S3::Bucket")
 	for _, bucket := range buckets {
 		_, n, _ := s11n.GetMapValue(bucket.Node, "Metadata")
@@ -170,10 +166,6 @@ func Run(n *yaml.Node, key string, stackName string, rootDir string) error {
 // will upload local assets to the bucket.
 func processMetadataAfter(template cft.Template, stackName string, rootDir string) error {
 
-	// For some reason Package created an extra document node
-	// (And CreateChangeSet is ok with this...?)
-	template.Node = template.Node.Content[0]
-
 	buckets := template.GetResourcesOfType("AWS::S3::Bucket")
 	for _, bucket := range buckets {
 		logicalId := bucket.LogicalId
diff --git a/test/webapp/site/js/app.js b/test/webapp/site/js/app.js
index 412f08f3..44d0e3bf 100644
--- a/test/webapp/site/js/app.js
+++ b/test/webapp/site/js/app.js
@@ -54,12 +54,12 @@ import * as restApi from "./rest-api"
             const d = data[i]
             const id = d.id.Value
             let foo = ""
-            if (d.foo) {
-                foo = d.foo.Value
+            if (d.Foo) {
+                foo = d.Foo.Value
             }
             let bar = ""
-            if (d.bar) {
-                bar = d.bar.Value
+            if (d.Bar) {
+                bar = d.Bar.Value
             }
             // Create a new table row
             const row = tbl.insertRow()
diff --git a/test/webapp/webapp-pkg.yaml b/test/webapp/webapp-pkg.yaml
index ffdca8ac..30e63508 100644
--- a/test/webapp/webapp-pkg.yaml
+++ b/test/webapp/webapp-pkg.yaml
@@ -134,7 +134,7 @@ Resources:
       Rain:
         EmptyOnDelete: true
         Content: site/dist
-        Version: 2
+        Version: 4
         DistributionLogicalId: SiteDistribution
         RunBefore:
           Command: buildsite.sh

From 0a6799ac8a5acaf37c9843cc8a042d69ea33ffc8 Mon Sep 17 00:00:00 2001
From: Eric Beard <ericzbeard@gmail.com>
Date: Fri, 8 Nov 2024 11:21:02 -0800
Subject: [PATCH 08/15] Downlad a package zip

---
 cft/pkg/download.go                       | 193 ++++++++++++++++++++++
 cft/pkg/module.go                         |  49 ++----
 cft/pkg/tmpl/.gitignore                   |   1 +
 cft/pkg/tmpl/pkg-alias-expect.yaml        |   4 +
 cft/pkg/tmpl/pkg-alias-module.yaml        |   4 +
 cft/pkg/tmpl/pkg-alias-template.yaml      |   8 +
 internal/aws/codeartifact/codeartifact.go |  77 +--------
 internal/cmd/module/install.go            |   8 +-
 8 files changed, 233 insertions(+), 111 deletions(-)
 create mode 100644 cft/pkg/download.go
 create mode 100644 cft/pkg/tmpl/.gitignore
 create mode 100644 cft/pkg/tmpl/pkg-alias-expect.yaml
 create mode 100644 cft/pkg/tmpl/pkg-alias-module.yaml
 create mode 100644 cft/pkg/tmpl/pkg-alias-template.yaml

diff --git a/cft/pkg/download.go b/cft/pkg/download.go
new file mode 100644
index 00000000..d126a7a7
--- /dev/null
+++ b/cft/pkg/download.go
@@ -0,0 +1,193 @@
+package pkg
+
+import (
+	"archive/zip"
+	"crypto/sha256"
+	"fmt"
+	"io"
+	"io/fs"
+	"net/http"
+	"net/url"
+	"os"
+	"path/filepath"
+	"strings"
+
+	"github.com/aws-cloudformation/rain/internal/config"
+	"github.com/google/uuid"
+)
+
+// DownloadFromZip retrieves a single file from a zip file hosted on a URI
+func DownloadFromZip(uriString string, verifyHash string, path string) ([]byte, error) {
+
+	config.Debugf("Downloading %s", uriString)
+	resp, err := http.Get(uriString)
+	if err != nil {
+		return nil, err
+	}
+	defer func(Body io.ReadCloser) {
+		err := Body.Close()
+		if err != nil {
+			config.Debugf("Error closing body: %v", err)
+		}
+	}(resp.Body)
+
+	u, err := url.Parse(uriString)
+	if err != nil {
+		return nil, err
+	}
+
+	filename := filepath.Base(u.Path)
+
+	// Save the asset content to a temp file
+	pFile, err := os.CreateTemp("", filename)
+	if err != nil {
+		return nil, err
+	}
+	defer func(pFile *os.File) {
+		err := pFile.Close()
+		if err != nil {
+			config.Debugf("Error closing file: %s", err)
+		}
+	}(pFile)
+
+	config.Debugf("Saving zip content to %s", pFile.Name())
+
+	// Write the asset content to the temp file
+	if _, err := io.Copy(pFile, resp.Body); err != nil {
+		return nil, err
+	}
+
+	// Seek to the beginning of the file
+	if _, err := pFile.Seek(0, 0); err != nil {
+		return nil, err
+	}
+
+	if verifyHash != "" {
+		// Create a sha256 hash of the asset content and verify it
+		hash := sha256.New()
+		// Read the contents of the temporary pFile and generate a sha256 hash
+		if _, err := io.Copy(hash, pFile); err != nil {
+			return nil, err
+		}
+		hashValue := hash.Sum(nil)
+
+		// Convert the hash value to a hex string
+		hashString := fmt.Sprintf("%x", hashValue)
+
+		config.Debugf("Hash value: %x", hashString)
+
+		// Reset pFile to the beginning
+		if _, err := pFile.Seek(0, 0); err != nil {
+			return nil, err
+		}
+
+		if verifyHash != hashString {
+			return nil, fmt.Errorf("hash does not match: %s != %s", verifyHash, hashString)
+		}
+	}
+
+	// Unzip the temp file
+	dir := filepath.Join(os.TempDir(), uuid.NewString())
+	err = Unzip(pFile, dir)
+	if err != nil {
+		return nil, err
+	}
+
+	content, err := os.ReadFile(filepath.Join(dir, path))
+	if err != nil {
+		return nil, err
+	}
+
+	return content, nil
+}
+
+// Unzip unzips a zip file to a destination directory
+func Unzip(f *os.File, dest string) error {
+	// Open a file reader
+	r, err := zip.OpenReader(f.Name())
+	if err != nil {
+		return err
+	}
+	defer func(r *zip.ReadCloser) {
+		err := r.Close()
+		if err != nil {
+			config.Debugf("Error closing zip reader: %s", err)
+		}
+	}(r)
+
+	// Iterate through the files in the archive,
+	// extracting each to the output directory
+	for _, f := range r.File {
+		rc, err := f.Open()
+		if err != nil {
+			return err
+		}
+		defer func(rc io.ReadCloser) {
+			err := rc.Close()
+			if err != nil {
+				config.Debugf("Error closing file: %s", err)
+			}
+		}(rc)
+
+		fpath := filepath.Join(dest, f.Name)
+		if f.FileInfo().IsDir() {
+			mode := fs.ModePerm
+			err := os.MkdirAll(fpath, mode)
+			if err != nil {
+				return err
+			}
+			config.Debugf("Created directory: %s with mode %x", fpath, mode)
+		} else {
+			var fdir string
+			if lastIndex := strings.LastIndex(fpath, string(os.PathSeparator)); lastIndex > -1 {
+				fdir = fpath[:lastIndex]
+				mode := fs.ModePerm
+				err := os.MkdirAll(fdir, mode)
+				if err != nil {
+					return err
+				}
+				config.Debugf("Created subdirectory: %s with mode %x", fdir, mode)
+			}
+
+			f, err := os.OpenFile(fpath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, f.Mode())
+			if err != nil {
+				return err
+			}
+			defer func(f *os.File) {
+				err := f.Close()
+				if err != nil {
+					config.Debugf("Error closing file: %s", err)
+				}
+			}(f)
+
+			_, err = io.Copy(f, rc)
+			if err != nil {
+				return err
+			}
+		}
+	}
+
+	return nil
+}
+
+// downloadModule downloads the file from the given URI and returns its content as a byte slice.
+func downloadModule(uri string) ([]byte, error) {
+	config.Debugf("Downloading %s", uri)
+	resp, err := http.Get(uri)
+	if err != nil {
+		return nil, err
+	}
+	defer func(Body io.ReadCloser) {
+		err := Body.Close()
+		if err != nil {
+			config.Debugf("Error closing body: %v", err)
+		}
+	}(resp.Body)
+
+	content, err := io.ReadAll(resp.Body)
+	if err != nil {
+		return nil, err
+	}
+
+	return content, nil
+}
diff --git a/cft/pkg/module.go b/cft/pkg/module.go
index f041fad0..b6c7ca97 100644
--- a/cft/pkg/module.go
+++ b/cft/pkg/module.go
@@ -4,8 +4,6 @@ package pkg
 import (
 	"errors"
 	"fmt"
-	"io"
-	"net/http"
 	"os"
 	"path/filepath"
 	"strings"
@@ -378,6 +376,7 @@ func resolveModuleSub(parentName string, prop *yaml.Node, sidx int, ctx *refctx)
 			}
 			sub += fmt.Sprintf("${%s.%s}", left, right)
 			needSub = true
+		// This should not be necessary since we process Rain constants earlier
 		//case parse.RAIN:
 		//	// Replace ${Rain::ConstantName} with template constant value
 		//	if ctx.constants == nil {
@@ -782,28 +781,6 @@ func processModule(
 	return true, nil
 }
 
-// downloadModule downloads the file from the given URI and returns its content as a byte slice.
-func downloadModule(uri string) ([]byte, error) {
-	config.Debugf("Downloading %s", uri)
-	resp, err := http.Get(uri)
-	if err != nil {
-		return nil, err
-	}
-	defer func(Body io.ReadCloser) {
-		err := Body.Close()
-		if err != nil {
-			config.Debugf("Error closing body: %v", err)
-		}
-	}(resp.Body)
-
-	content, err := io.ReadAll(resp.Body)
-	if err != nil {
-		return nil, err
-	}
-
-	return content, nil
-}
-
 func checkPackageAlias(t cft.Template, uri string) *cft.PackageAlias {
 	config.Debugf("checkPackageAlias uri: %s", uri)
 	tokens := strings.Split(uri, "/")
@@ -848,23 +825,25 @@ func module(ctx *directiveContext) (bool, error) {
 	isZip := false
 	if packageAlias != nil {
 		config.Debugf("Found package alias: %+v", packageAlias)
-		uri = strings.Replace(uri, packageAlias.Alias, packageAlias.Location, 1)
-		config.Debugf("uri is now %s", uri)
-		config.Debugf("baseUri is %s", baseUri)
-
-		// This might be a zipped directory.
-		if strings.HasSuffix(uri, ".zip") {
+		path := strings.Replace(uri, packageAlias.Alias+"/", "", 1)
+		config.Debugf("path is %s", path)
+		if strings.HasSuffix(packageAlias.Location, ".zip") {
 			// Unzip, verify hash if there is one, and put the files in memory
 			isZip = true
-			// TODO
+			content, err = DownloadFromZip(packageAlias.Location, packageAlias.Hash, path)
+			if err != nil {
+				return false, err
+			}
+		} else {
+			uri = strings.Replace(uri, packageAlias.Alias, packageAlias.Location, 1)
+			config.Debugf("uri is now %s", uri)
+			config.Debugf("baseUri is %s", baseUri)
 		}
 	}
 
-	// Is this a local file or a URL or an in memory file system?
+	// Is this a local file or a URL or did we already unzip a package?
 	if isZip {
-
-		// TODO - Get content from in memory unzipped files
-
+		config.Debugf("Got content from a zipped module package: %s", string(content))
 	} else if strings.HasPrefix(uri, "https://") {
 
 		content, err = downloadModule(uri)
diff --git a/cft/pkg/tmpl/.gitignore b/cft/pkg/tmpl/.gitignore
new file mode 100644
index 00000000..2f472a77
--- /dev/null
+++ b/cft/pkg/tmpl/.gitignore
@@ -0,0 +1 @@
+modules.zip
diff --git a/cft/pkg/tmpl/pkg-alias-expect.yaml b/cft/pkg/tmpl/pkg-alias-expect.yaml
new file mode 100644
index 00000000..6522457c
--- /dev/null
+++ b/cft/pkg/tmpl/pkg-alias-expect.yaml
@@ -0,0 +1,4 @@
+Resources:
+  foobar:
+    Type: AWS::S3::Bucket
+
diff --git a/cft/pkg/tmpl/pkg-alias-module.yaml b/cft/pkg/tmpl/pkg-alias-module.yaml
new file mode 100644
index 00000000..d9cd0ae9
--- /dev/null
+++ b/cft/pkg/tmpl/pkg-alias-module.yaml
@@ -0,0 +1,4 @@
+Resources:
+  bar:
+    Type: AWS::S3::Bucket
+
diff --git a/cft/pkg/tmpl/pkg-alias-template.yaml b/cft/pkg/tmpl/pkg-alias-template.yaml
new file mode 100644
index 00000000..c5b8bdf3
--- /dev/null
+++ b/cft/pkg/tmpl/pkg-alias-template.yaml
@@ -0,0 +1,8 @@
+Rain:
+  Packages:
+    abc:
+      Location: http://localhost:3000/modules.zip
+Resources:
+  foo:
+    Type: $abc/pkg-alias-module.yaml
+
diff --git a/internal/aws/codeartifact/codeartifact.go b/internal/aws/codeartifact/codeartifact.go
index d6d5e872..bc6ae40a 100644
--- a/internal/aws/codeartifact/codeartifact.go
+++ b/internal/aws/codeartifact/codeartifact.go
@@ -8,16 +8,16 @@ import (
 	"crypto/sha256"
 	"errors"
 	"fmt"
-	"github.com/aws-cloudformation/rain/internal/console/spinner"
-	"github.com/aws/aws-sdk-go-v2/aws"
-	"github.com/aws/aws-sdk-go-v2/service/codeartifact/types"
 	"io"
-	"io/fs"
 	"os"
 	"path/filepath"
 	"strconv"
 	"strings"
 
+	"github.com/aws-cloudformation/rain/internal/console/spinner"
+	"github.com/aws/aws-sdk-go-v2/aws"
+	"github.com/aws/aws-sdk-go-v2/service/codeartifact/types"
+
 	rainaws "github.com/aws-cloudformation/rain/internal/aws"
 	"github.com/aws-cloudformation/rain/internal/config"
 	"github.com/aws/aws-sdk-go-v2/service/codeartifact"
@@ -354,75 +354,6 @@ func Publish(packageInfo *PackageInfo) error {
 	return nil
 }
 
-// Unzip unzips a zip file to a destination directory
-func Unzip(f *os.File, dest string) error {
-	// Open a file reader
-	r, err := zip.OpenReader(f.Name())
-	if err != nil {
-		return err
-	}
-	defer func(r *zip.ReadCloser) {
-		err := r.Close()
-		if err != nil {
-			config.Debugf("Error closing zip reader: %s", err)
-		}
-	}(r)
-
-	// Iterate through the files in the archive,
-	// extracting each to the output directory
-	for _, f := range r.File {
-		rc, err := f.Open()
-		if err != nil {
-			return err
-		}
-		defer func(rc io.ReadCloser) {
-			err := rc.Close()
-			if err != nil {
-				config.Debugf("Error closing file: %s", err)
-			}
-		}(rc)
-
-		fpath := filepath.Join(dest, f.Name)
-		if f.FileInfo().IsDir() {
-			mode := fs.ModePerm
-			err := os.MkdirAll(fpath, mode)
-			if err != nil {
-				return err
-			}
-			config.Debugf("Created directory: %s with mode %x", fpath, mode)
-		} else {
-			var fdir string
-			if lastIndex := strings.LastIndex(fpath, string(os.PathSeparator)); lastIndex > -1 {
-				fdir = fpath[:lastIndex]
-				mode := fs.ModePerm
-				err := os.MkdirAll(fdir, mode)
-				if err != nil {
-					return err
-				}
-				config.Debugf("Created subdirectory: %s with mode %x", fdir, mode)
-			}
-
-			f, err := os.OpenFile(fpath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, f.Mode())
-			if err != nil {
-				return err
-			}
-			defer func(f *os.File) {
-				err := f.Close()
-				if err != nil {
-					config.Debugf("Error closing file: %s", err)
-				}
-			}(f)
-
-			_, err = io.Copy(f, rc)
-			if err != nil {
-				return err
-			}
-		}
-	}
-
-	return nil
-}
-
 // GetAssetHashForPackage returns the hash of the asset for a package
 func GetAssetHashForPackage(packageInfo *PackageInfo) (string, error) {
 
diff --git a/internal/cmd/module/install.go b/internal/cmd/module/install.go
index 7db8d475..b4302d22 100644
--- a/internal/cmd/module/install.go
+++ b/internal/cmd/module/install.go
@@ -3,13 +3,15 @@ package module
 import (
 	"crypto/sha256"
 	"fmt"
+	"io"
+	"os"
+
+	"github.com/aws-cloudformation/rain/cft/pkg"
 	"github.com/aws-cloudformation/rain/internal/aws/codeartifact"
 	"github.com/aws-cloudformation/rain/internal/config"
 	"github.com/aws-cloudformation/rain/internal/console"
 	"github.com/aws-cloudformation/rain/internal/console/spinner"
 	"github.com/spf13/cobra"
-	"io"
-	"os"
 )
 
 func install(cmd *cobra.Command, args []string) {
@@ -129,7 +131,7 @@ func install(cmd *cobra.Command, args []string) {
 	}
 
 	// Unzip pFile into the new package directory
-	err = codeartifact.Unzip(pFile, packageInfo.DirectoryPath)
+	err = pkg.Unzip(pFile, packageInfo.DirectoryPath)
 	if err != nil {
 		panic(err)
 	}

From a2653b5e1e42813c3b0f077b51f55eb2c2a13af5 Mon Sep 17 00:00:00 2001
From: Eric Beard <ericzbeard@gmail.com>
Date: Fri, 8 Nov 2024 13:02:07 -0800
Subject: [PATCH 09/15] Fix module release

---
 .github/workflows/modules.yml | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/.github/workflows/modules.yml b/.github/workflows/modules.yml
index 174a5749..3d9db5aa 100644
--- a/.github/workflows/modules.yml
+++ b/.github/workflows/modules.yml
@@ -48,14 +48,8 @@ jobs:
       - run: |
           set -x
           (echo "${GITHUB_REF##*/}"; echo; git cherry -v "$(git describe --abbrev=0 HEAD^)" | cut -d" " -f3-) > CHANGELOG
-          assets=()
-          for zip in ./dist/*.zip; do
-            assets+=("$zip")
-          done
-          for hash in ./dist/*.rsa256; do
-            assets+=("$hash")
-          done
-          gh release upload "${GITHUB_REF##*/}" "${assets[@]}"
+          gh release upload "${GITHUB_REF##*/}" dist/modules.zip
+          gh release upload "${GITHUB_REF##*/}" dist/modules.sha256
         env:
           GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
 

From 1b2f8e564e713d01f954d7934bc7b29c2b56112e Mon Sep 17 00:00:00 2001
From: Eric Beard <ericzbeard@gmail.com>
Date: Fri, 8 Nov 2024 13:28:11 -0800
Subject: [PATCH 10/15] Fix hash check

---
 .github/workflows/modules.sh  |  2 +-
 cft/pkg/download.go           | 33 +++++++++++++++++++++++++++++++--
 test/templates/pkg-alias.yaml |  9 +++++++++
 3 files changed, 41 insertions(+), 3 deletions(-)
 create mode 100644 test/templates/pkg-alias.yaml

diff --git a/.github/workflows/modules.sh b/.github/workflows/modules.sh
index 774ae1f1..5b02a9da 100755
--- a/.github/workflows/modules.sh
+++ b/.github/workflows/modules.sh
@@ -5,4 +5,4 @@ set -eoux pipefail
 # Zip up the modules directory and create a sha256 hash
 mkdir -p dist
 zip -r dist/modules.zip modules
-sha256sum -b dist/modules.zip > dist/modules.sha256
+sha256sum -b dist/modules.zip | cut -d " " -f 1  > dist/modules.sha256
diff --git a/cft/pkg/download.go b/cft/pkg/download.go
index d126a7a7..3d5f1a57 100644
--- a/cft/pkg/download.go
+++ b/cft/pkg/download.go
@@ -16,6 +16,29 @@ import (
 	"github.com/google/uuid"
 )
 
+// Downloads the hash file and returns the contents
+func downloadHash(uri string) (string, error) {
+
+	config.Debugf("Downloading %s", uri)
+	resp, err := http.Get(uri)
+	if err != nil {
+		return "", err
+	}
+	defer func(Body io.ReadCloser) {
+		err := Body.Close()
+		if err != nil {
+			config.Debugf("Error closing body: %v", err)
+		}
+	}(resp.Body)
+
+	data, err := io.ReadAll(resp.Body)
+	if err != nil {
+		return "", nil
+	}
+
+	return string(data), nil
+}
+
 // DownloadFromZip retrieves a single file from a zip file hosted on a URI
 func DownloadFromZip(uriString string, verifyHash string, path string) ([]byte, error) {
 
@@ -81,8 +104,14 @@ func DownloadFromZip(uriString string, verifyHash string, path string) ([]byte,
 			return nil, err
 		}
 
-		if verifyHash != hashString {
-			return nil, fmt.Errorf("hash does not match: %s != %s", verifyHash, hashString)
+		// Download the hash
+		originalHash, err := downloadHash(verifyHash)
+		if err != nil {
+			return nil, err
+		}
+
+		if originalHash != hashString {
+			return nil, fmt.Errorf("hash does not match: %s != %s", originalHash, hashString)
 		}
 	}
 
diff --git a/test/templates/pkg-alias.yaml b/test/templates/pkg-alias.yaml
new file mode 100644
index 00000000..fea65740
--- /dev/null
+++ b/test/templates/pkg-alias.yaml
@@ -0,0 +1,9 @@
+Rain:
+  Packages:
+    abc:
+      Location: https://github.com/ericzbeard/rain/releases/download/m0.1.0-a/modules.zip
+      Hash: https://github.com/ericzbeard/rain/releases/download/m0.1.0-a/modules.sha256
+Resources:
+  foo:
+    Type: $abc/simple-bucket.yaml
+

From bad53596300aff00d3af6b66bc36b06a4eeaf2a2 Mon Sep 17 00:00:00 2001
From: Eric Beard <ericzbeard@gmail.com>
Date: Fri, 8 Nov 2024 13:56:33 -0800
Subject: [PATCH 11/15] Fix zip directory

---
 .github/workflows/modules.sh  | 5 +++--
 cft/pkg/download.go           | 6 +++++-
 test/templates/pkg-alias.yaml | 4 ++--
 3 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/.github/workflows/modules.sh b/.github/workflows/modules.sh
index 5b02a9da..a4123f6b 100755
--- a/.github/workflows/modules.sh
+++ b/.github/workflows/modules.sh
@@ -4,5 +4,6 @@ set -eoux pipefail
 
 # Zip up the modules directory and create a sha256 hash
 mkdir -p dist
-zip -r dist/modules.zip modules
-sha256sum -b dist/modules.zip | cut -d " " -f 1  > dist/modules.sha256
+cd modules
+zip -r ../dist/modules.zip *
+sha256sum -b ../dist/modules.zip | cut -d " " -f 1  > ../dist/modules.sha256
diff --git a/cft/pkg/download.go b/cft/pkg/download.go
index 3d5f1a57..21667cca 100644
--- a/cft/pkg/download.go
+++ b/cft/pkg/download.go
@@ -36,7 +36,9 @@ func downloadHash(uri string) (string, error) {
 		return "", nil
 	}
 
-	return string(data), nil
+	retval := string(data)
+	retval = strings.Trim(retval, " \n")
+	return retval, nil
 }
 
 // DownloadFromZip retrieves a single file from a zip file hosted on a URI
@@ -61,6 +63,8 @@ func DownloadFromZip(uriString string, verifyHash string, path string) ([]byte,
 
 	filename := filepath.Base(u.Path)
 
+	config.Debugf("filename is %s", filename)
+
 	// Save the asset content to a temp file
 	pFile, err := os.CreateTemp("", filename)
 	if err != nil {
diff --git a/test/templates/pkg-alias.yaml b/test/templates/pkg-alias.yaml
index fea65740..897986a9 100644
--- a/test/templates/pkg-alias.yaml
+++ b/test/templates/pkg-alias.yaml
@@ -1,8 +1,8 @@
 Rain:
   Packages:
     abc:
-      Location: https://github.com/ericzbeard/rain/releases/download/m0.1.0-a/modules.zip
-      Hash: https://github.com/ericzbeard/rain/releases/download/m0.1.0-a/modules.sha256
+      Location: https://github.com/ericzbeard/rain/releases/download/m0.1.0-b/modules.zip
+      Hash: https://github.com/ericzbeard/rain/releases/download/m0.1.0-b/modules.sha256
 Resources:
   foo:
     Type: $abc/simple-bucket.yaml

From 4000b5398f05aacf2b9799eccba5d43bdac86064 Mon Sep 17 00:00:00 2001
From: Eric Beard <ericzbeard@gmail.com>
Date: Fri, 8 Nov 2024 14:06:10 -0800
Subject: [PATCH 12/15] Finish up module packaging

---
 cft/pkg/download.go           | 6 ------
 cft/pkg/module.go             | 7 -------
 cft/pkg/pkg.go                | 4 ----
 test/templates/pkg-alias.yaml | 6 +++---
 4 files changed, 3 insertions(+), 20 deletions(-)

diff --git a/cft/pkg/download.go b/cft/pkg/download.go
index 21667cca..dcf3dedc 100644
--- a/cft/pkg/download.go
+++ b/cft/pkg/download.go
@@ -63,8 +63,6 @@ func DownloadFromZip(uriString string, verifyHash string, path string) ([]byte,
 
 	filename := filepath.Base(u.Path)
 
-	config.Debugf("filename is %s", filename)
-
 	// Save the asset content to a temp file
 	pFile, err := os.CreateTemp("", filename)
 	if err != nil {
@@ -101,8 +99,6 @@ func DownloadFromZip(uriString string, verifyHash string, path string) ([]byte,
 		// Convert the hash value to a hex string
 		hashString := fmt.Sprintf("%x", hashValue)
 
-		config.Debugf("Hash value: %x", hashString)
-
 		// Reset pFile to the beginning
 		if _, err := pFile.Seek(0, 0); err != nil {
 			return nil, err
@@ -169,7 +165,6 @@ func Unzip(f *os.File, dest string) error {
 			if err != nil {
 				return err
 			}
-			config.Debugf("Created directory: %s with mode %x", fpath, mode)
 		} else {
 			var fdir string
 			if lastIndex := strings.LastIndex(fpath, string(os.PathSeparator)); lastIndex > -1 {
@@ -179,7 +174,6 @@ func Unzip(f *os.File, dest string) error {
 				if err != nil {
 					return err
 				}
-				config.Debugf("Created subdirectory: %s with mode %x", fdir, mode)
 			}
 
 			f, err := os.OpenFile(fpath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, f.Mode())
diff --git a/cft/pkg/module.go b/cft/pkg/module.go
index b6c7ca97..fbd8aee2 100644
--- a/cft/pkg/module.go
+++ b/cft/pkg/module.go
@@ -782,7 +782,6 @@ func processModule(
 }
 
 func checkPackageAlias(t cft.Template, uri string) *cft.PackageAlias {
-	config.Debugf("checkPackageAlias uri: %s", uri)
 	tokens := strings.Split(uri, "/")
 	if len(tokens) > 1 {
 		// See if this is one of the template package aliases
@@ -889,8 +888,6 @@ func module(ctx *directiveContext) (bool, error) {
 				path = filepath.Join(root, path)
 			}
 
-			config.Debugf("abs path: %v", path)
-
 			info, err := os.Stat(path)
 			if err != nil {
 				return false, err
@@ -931,11 +928,7 @@ func module(ctx *directiveContext) (bool, error) {
 	moduleAsTemplate := cft.Template{Node: &moduleNode}
 
 	// Read things like Constants
-	config.Debugf("About to processRainSection in %s", uri)
 	processRainSection(&moduleAsTemplate)
-	for k, v := range moduleAsTemplate.Constants {
-		config.Debugf("%s = %s", k, v.Value)
-	}
 
 	if moduleAsTemplate.Constants != nil {
 		replaceTemplateConstants(moduleAsTemplate.Node, moduleAsTemplate.Constants)
diff --git a/cft/pkg/pkg.go b/cft/pkg/pkg.go
index 99dd0b52..4fba2074 100644
--- a/cft/pkg/pkg.go
+++ b/cft/pkg/pkg.go
@@ -86,11 +86,9 @@ func processRainSection(t *cft.Template) bool {
 	rainNode, err := t.GetSection(cft.Rain)
 	if err != nil {
 		// This is okay, not all templates have a Rain section
-		config.Debugf("Unable to get Rain section: %v", err)
 		return false
 	}
 
-	config.Debugf("Rain node: %s", node.ToSJson(rainNode))
 	// Process constants in order, since they can refer to previous ones
 	_, c, _ := s11n.GetMapValue(rainNode, "Constants")
 	if c != nil {
@@ -194,11 +192,9 @@ func Template(t cft.Template, rootDir string, fs *embed.FS) (cft.Template, error
 			return t, err
 		}
 		if changedThisPass {
-			config.Debugf("Need another pass: %d", passes)
 			changed = true
 		}
 		if !changedThisPass {
-			config.Debugf("No changes this pass: %d", passes)
 			break
 		}
 		if passes > maxPasses {
diff --git a/test/templates/pkg-alias.yaml b/test/templates/pkg-alias.yaml
index 897986a9..74d8fb60 100644
--- a/test/templates/pkg-alias.yaml
+++ b/test/templates/pkg-alias.yaml
@@ -1,9 +1,9 @@
 Rain:
   Packages:
     abc:
-      Location: https://github.com/ericzbeard/rain/releases/download/m0.1.0-b/modules.zip
-      Hash: https://github.com/ericzbeard/rain/releases/download/m0.1.0-b/modules.sha256
+      Location: https://github.com/ericzbeard/rain/releases/download/m0.1.0-c/modules.zip
+      Hash: https://github.com/ericzbeard/rain/releases/download/m0.1.0-c/modules.sha256
 Resources:
   foo:
-    Type: $abc/simple-bucket.yaml
+    Type: $abc/encrypted-bucket.yaml
 

From ea50edf7254a39207cda714db33ad1f21c5e8d2b Mon Sep 17 00:00:00 2001
From: Eric Beard <ericzbeard@gmail.com>
Date: Fri, 8 Nov 2024 14:29:56 -0800
Subject: [PATCH 13/15] Docs

---
 README.md                     | 96 ++++++++++++++++++++++++++++++-----
 docs/README.tmpl              | 19 ++++---
 docs/index.md                 |  2 +-
 docs/rain_bootstrap.md        |  2 +-
 docs/rain_build.md            |  2 +-
 docs/rain_cat.md              |  2 +-
 docs/rain_cc.md               |  2 +-
 docs/rain_cc_deploy.md        |  2 +-
 docs/rain_cc_drift.md         |  2 +-
 docs/rain_cc_rm.md            |  2 +-
 docs/rain_cc_state.md         |  2 +-
 docs/rain_console.md          |  2 +-
 docs/rain_deploy.md           |  2 +-
 docs/rain_diff.md             |  2 +-
 docs/rain_fmt.md              |  2 +-
 docs/rain_forecast.md         |  2 +-
 docs/rain_info.md             |  2 +-
 docs/rain_logs.md             |  2 +-
 docs/rain_ls.md               |  2 +-
 docs/rain_merge.md            |  2 +-
 docs/rain_module.md           |  2 +-
 docs/rain_module_bootstrap.md |  2 +-
 docs/rain_module_install.md   |  2 +-
 docs/rain_module_publish.md   |  2 +-
 docs/rain_pkg.md              |  2 +-
 docs/rain_rm.md               |  2 +-
 docs/rain_stackset.md         |  2 +-
 docs/rain_stackset_deploy.md  |  2 +-
 docs/rain_stackset_ls.md      |  2 +-
 docs/rain_stackset_rm.md      |  2 +-
 docs/rain_tree.md             |  2 +-
 docs/rain_watch.md            |  2 +-
 32 files changed, 126 insertions(+), 49 deletions(-)

diff --git a/README.md b/README.md
index d5257b8c..7c70c4d1 100644
--- a/README.md
+++ b/README.md
@@ -308,14 +308,11 @@ See `test/webapp/README.md` for a complete example of using these commands with
 #### Module
 
 The `!Rain::Module` directive is an experimental feature that allows you to
-create local modules of reuseable code that can be inserted into templates. A
-rain module is similar in some ways to a CDK construct, in that a module can
-extend existing resources, allowing the user of the module to override
-properties. For example, your module could extend an S3 bucket to provide a
-default implementation that passes static security scans. Users of the module
-would inherit these best practices by default, but they would still have the
-ability to configure any of the original properties on `AWS::S3::Bucket`, in
-addition to the properties defined as module parameters.
+create local modules of reuseable code that can be inserted into templates. 
+Rain modules are basically just CloudFormation templates, with a Parameters 
+section that corresponds to the Properties that a consumer will set when 
+using the module. Rain modules are very flexible, since you can override 
+any of the resource properties from the parent template.
 
 In order to use this feature, you have to acknowledge that it's experimental by
 adding a flag on the command line:
@@ -333,8 +330,7 @@ directives.
 A sample module:
 
 ```yaml
-Description: |
-  This module extends AWS::S3::Bucket
+Description: This module creates a compliant bucket, along with a second bucket to store access logs
 
 Parameters:
   LogBucketName:
@@ -449,13 +445,89 @@ Resources:
         RestrictPublicBuckets: true
 ```
 
-### Module package publishing
+### Publish modules to CodeArtifact 
 
 Rain integrates with AWS CodeArtifact to enable an experience similar to npm
 publish and install. A directory that includes Rain module YAML files can be
-packaged up with `rain module publish`, and then the package can be installed
+packaged up with `rain module publish`, and then the directory can be installed
 by developers with `rain module install`.
 
+### Module packaging
+
+You can reference a collection of Rain modules with an alias inside of the
+parent template. Add a `Rain` section to the template to configure the package
+alias. There's nothing special about a package, it's just an alias to a
+directory or a zip file. A zip file can also have a sha256 hash associated with
+it to verify the contents.
+
+```yaml
+Rain:
+  Packages:
+    aws:
+      Location: https://github.com/aws-cloudformation/rain/modules
+    xyz:
+      Location: ./my-modules
+    abc:
+      Location: https://github.com/aws-cloudformation/rain/releases/tag/m0.1.0/modules.zip
+      Hash: https://github.com/aws-cloudformation/rain/releases/tag/m0.1.0/modules.sha256
+
+Resources:
+  Foo:
+    Type: !Rain::Module aws/foo.yaml
+
+  Bar:
+    Type: !Rain::Module xyz/bar.yaml
+
+  Baz:
+    Type: $abc/baz.yaml
+    # Shorthand for !Rain::Module abc/baz.yaml
+```
+
+A module package is published and released from this repository separately from
+the Rain binary release. This allows the package to be referenced by version
+numbers using tags, such as `m0.1.0` as shown in the example above. The major
+version number will be incremented if any breaking changes are introduced to
+the modules. The available modules in the release package are listed below. 
+
+Treat these modules as samples to be used as a proof-of-concept for building your 
+own module packages.
+
+#### simple-vpc.yaml
+
+A VPC with just two availability zones. This module is useful for POCs and simple projects.
+
+#### encrypted-bucket.yaml
+
+A simple bucket with encryption enabled and public access blocked
+
+#### compliant-bucket.yaml
+
+A bucket, plus extra buckets for access logs and replication and a bucket policy that should pass most typical compliance checks.
+
+#### bucket-policy.yaml
+
+A bucket policy that denies requests not made with TLS.
+
+#### load-balancer.yaml
+
+An ELBv2 load balancer
+
+#### static-site.yaml
+
+An S3 bucket and a CloudFront distribution to host content for a web site
+
+#### cognito.yaml
+
+A Cognito User Pool and associated resources
+
+#### rest-api.yaml
+
+An API Gateway REST API
+
+#### api-resource.yaml
+
+A Lambda function and associated API Gateway resources
+
 ### Gantt Chart
 
 Output a chart to an HTML file that you can view with a browser to look at how long stack operations take for each resource.
diff --git a/docs/README.tmpl b/docs/README.tmpl
index 01301e88..81831c77 100644
--- a/docs/README.tmpl
+++ b/docs/README.tmpl
@@ -417,18 +417,20 @@ Resources:
         RestrictPublicBuckets: true
 ```
 
-### Module publishing
+### Publish modules to CodeArtifact 
 
 Rain integrates with AWS CodeArtifact to enable an experience similar to npm
 publish and install. A directory that includes Rain module YAML files can be
 packaged up with `rain module publish`, and then the directory can be installed
 by developers with `rain module install`.
 
-In addition to the CodeArtifact integration, you can reference a collection of 
-Rain modules with an alias inside of the parent template. Add a `Rain` section 
-to the template to configure the package alias. There's nothing special about 
-a package, it's just an alias to a directory or a zip file. A zip file can also 
-have a sha256 hash associated with it to verify the contents.
+### Module packaging
+
+You can reference a collection of Rain modules with an alias inside of the
+parent template. Add a `Rain` section to the template to configure the package
+alias. There's nothing special about a package, it's just an alias to a
+directory or a zip file. A zip file can also have a sha256 hash associated with
+it to verify the contents.
 
 ```yaml
 Rain:
@@ -449,7 +451,7 @@ Resources:
     Type: !Rain::Module xyz/bar.yaml
 
   Baz:
-    Type: $abc/baz 
+    Type: $abc/baz.yaml
     # Shorthand for !Rain::Module abc/baz.yaml
 ```
 
@@ -459,6 +461,9 @@ numbers using tags, such as `m0.1.0` as shown in the example above. The major
 version number will be incremented if any breaking changes are introduced to
 the modules. The available modules in the release package are listed below. 
 
+Treat these modules as samples to be used as a proof-of-concept for building your 
+own module packages.
+
 #### simple-vpc.yaml
 
 A VPC with just two availability zones. This module is useful for POCs and simple projects.
diff --git a/docs/index.md b/docs/index.md
index a1eab3e2..d53fa776 100644
--- a/docs/index.md
+++ b/docs/index.md
@@ -36,4 +36,4 @@ Rain is a command line tool for working with AWS CloudFormation templates and st
 * [rain tree](rain_tree.md)	 - Find dependencies of Resources and Outputs in a local template
 * [rain watch](rain_watch.md)	 - Display an updating view of a CloudFormation stack
 
-###### Auto generated by spf13/cobra on 31-Oct-2024
+###### Auto generated by spf13/cobra on 8-Nov-2024
diff --git a/docs/rain_bootstrap.md b/docs/rain_bootstrap.md
index bf4a5661..522db3b8 100644
--- a/docs/rain_bootstrap.md
+++ b/docs/rain_bootstrap.md
@@ -33,4 +33,4 @@ rain bootstrap
 
 * [rain](index.md)	 - 
 
-###### Auto generated by spf13/cobra on 31-Oct-2024
+###### Auto generated by spf13/cobra on 8-Nov-2024
diff --git a/docs/rain_build.md b/docs/rain_build.md
index 62609e73..9bdec880 100644
--- a/docs/rain_build.md
+++ b/docs/rain_build.md
@@ -41,4 +41,4 @@ rain build [<resource type>] or <prompt>
 
 * [rain](index.md)	 - 
 
-###### Auto generated by spf13/cobra on 31-Oct-2024
+###### Auto generated by spf13/cobra on 8-Nov-2024
diff --git a/docs/rain_cat.md b/docs/rain_cat.md
index ae9764a2..a3bf3197 100644
--- a/docs/rain_cat.md
+++ b/docs/rain_cat.md
@@ -35,4 +35,4 @@ rain cat <stack>
 
 * [rain](index.md)	 - 
 
-###### Auto generated by spf13/cobra on 31-Oct-2024
+###### Auto generated by spf13/cobra on 8-Nov-2024
diff --git a/docs/rain_cc.md b/docs/rain_cc.md
index 45bf363b..cbab88ef 100644
--- a/docs/rain_cc.md
+++ b/docs/rain_cc.md
@@ -33,4 +33,4 @@ You must pass the --experimental (-x) flag to use this command, to acknowledge t
 * [rain cc rm](rain_cc_rm.md)	 - Delete a deployment created by cc deploy (Experimental!)
 * [rain cc state](rain_cc_state.md)	 - Download the state file for a template deployed with cc deploy
 
-###### Auto generated by spf13/cobra on 31-Oct-2024
+###### Auto generated by spf13/cobra on 8-Nov-2024
diff --git a/docs/rain_cc_deploy.md b/docs/rain_cc_deploy.md
index 6b602f26..7c092307 100644
--- a/docs/rain_cc_deploy.md
+++ b/docs/rain_cc_deploy.md
@@ -40,4 +40,4 @@ rain cc deploy <template> <name>
 
 * [rain cc](rain_cc.md)	 - Interact with templates using Cloud Control API instead of CloudFormation
 
-###### Auto generated by spf13/cobra on 31-Oct-2024
+###### Auto generated by spf13/cobra on 8-Nov-2024
diff --git a/docs/rain_cc_drift.md b/docs/rain_cc_drift.md
index 9851eb70..f61c8a31 100644
--- a/docs/rain_cc_drift.md
+++ b/docs/rain_cc_drift.md
@@ -33,4 +33,4 @@ rain cc drift <name>
 
 * [rain cc](rain_cc.md)	 - Interact with templates using Cloud Control API instead of CloudFormation
 
-###### Auto generated by spf13/cobra on 31-Oct-2024
+###### Auto generated by spf13/cobra on 8-Nov-2024
diff --git a/docs/rain_cc_rm.md b/docs/rain_cc_rm.md
index c1233a6c..677f6a6c 100644
--- a/docs/rain_cc_rm.md
+++ b/docs/rain_cc_rm.md
@@ -33,4 +33,4 @@ rain cc rm <name>
 
 * [rain cc](rain_cc.md)	 - Interact with templates using Cloud Control API instead of CloudFormation
 
-###### Auto generated by spf13/cobra on 31-Oct-2024
+###### Auto generated by spf13/cobra on 8-Nov-2024
diff --git a/docs/rain_cc_state.md b/docs/rain_cc_state.md
index 4faaf8e6..91090f26 100644
--- a/docs/rain_cc_state.md
+++ b/docs/rain_cc_state.md
@@ -33,4 +33,4 @@ rain cc state <name>
 
 * [rain cc](rain_cc.md)	 - Interact with templates using Cloud Control API instead of CloudFormation
 
-###### Auto generated by spf13/cobra on 31-Oct-2024
+###### Auto generated by spf13/cobra on 8-Nov-2024
diff --git a/docs/rain_console.md b/docs/rain_console.md
index 42f61d03..a81062fc 100644
--- a/docs/rain_console.md
+++ b/docs/rain_console.md
@@ -39,4 +39,4 @@ rain console [stack]
 
 * [rain](index.md)	 - 
 
-###### Auto generated by spf13/cobra on 31-Oct-2024
+###### Auto generated by spf13/cobra on 8-Nov-2024
diff --git a/docs/rain_deploy.md b/docs/rain_deploy.md
index ea6baca9..b739df38 100644
--- a/docs/rain_deploy.md
+++ b/docs/rain_deploy.md
@@ -87,4 +87,4 @@ rain deploy <template> [stack]
 
 * [rain](index.md)	 - 
 
-###### Auto generated by spf13/cobra on 31-Oct-2024
+###### Auto generated by spf13/cobra on 8-Nov-2024
diff --git a/docs/rain_diff.md b/docs/rain_diff.md
index d7dd1935..a38c6b91 100644
--- a/docs/rain_diff.md
+++ b/docs/rain_diff.md
@@ -28,4 +28,4 @@ rain diff <from> <to>
 
 * [rain](index.md)	 - 
 
-###### Auto generated by spf13/cobra on 31-Oct-2024
+###### Auto generated by spf13/cobra on 8-Nov-2024
diff --git a/docs/rain_fmt.md b/docs/rain_fmt.md
index 818e3788..b37d2946 100644
--- a/docs/rain_fmt.md
+++ b/docs/rain_fmt.md
@@ -37,4 +37,4 @@ rain fmt <filename>...
 
 * [rain](index.md)	 - 
 
-###### Auto generated by spf13/cobra on 31-Oct-2024
+###### Auto generated by spf13/cobra on 8-Nov-2024
diff --git a/docs/rain_forecast.md b/docs/rain_forecast.md
index df0d4b42..7d5a9739 100644
--- a/docs/rain_forecast.md
+++ b/docs/rain_forecast.md
@@ -55,4 +55,4 @@ rain forecast --experimental <template> [stackName]
 
 * [rain](index.md)	 - 
 
-###### Auto generated by spf13/cobra on 31-Oct-2024
+###### Auto generated by spf13/cobra on 8-Nov-2024
diff --git a/docs/rain_info.md b/docs/rain_info.md
index ab5efb94..210ecc51 100644
--- a/docs/rain_info.md
+++ b/docs/rain_info.md
@@ -30,4 +30,4 @@ rain info
 
 * [rain](index.md)	 - 
 
-###### Auto generated by spf13/cobra on 31-Oct-2024
+###### Auto generated by spf13/cobra on 8-Nov-2024
diff --git a/docs/rain_logs.md b/docs/rain_logs.md
index 5d018ae3..1504d543 100644
--- a/docs/rain_logs.md
+++ b/docs/rain_logs.md
@@ -37,4 +37,4 @@ rain logs <stack> (<resource>)
 
 * [rain](index.md)	 - 
 
-###### Auto generated by spf13/cobra on 31-Oct-2024
+###### Auto generated by spf13/cobra on 8-Nov-2024
diff --git a/docs/rain_ls.md b/docs/rain_ls.md
index 798035c4..37d4dad3 100644
--- a/docs/rain_ls.md
+++ b/docs/rain_ls.md
@@ -31,4 +31,4 @@ rain ls <stack> [changeset]
 
 * [rain](index.md)	 - 
 
-###### Auto generated by spf13/cobra on 31-Oct-2024
+###### Auto generated by spf13/cobra on 8-Nov-2024
diff --git a/docs/rain_merge.md b/docs/rain_merge.md
index d6dc9eca..20f3345a 100644
--- a/docs/rain_merge.md
+++ b/docs/rain_merge.md
@@ -30,4 +30,4 @@ rain merge <template> <template> ...
 
 * [rain](index.md)	 - 
 
-###### Auto generated by spf13/cobra on 31-Oct-2024
+###### Auto generated by spf13/cobra on 8-Nov-2024
diff --git a/docs/rain_module.md b/docs/rain_module.md
index 76d2059a..693376b6 100644
--- a/docs/rain_module.md
+++ b/docs/rain_module.md
@@ -31,4 +31,4 @@ The rain module command can be used to publish modules to CodeArtifact, and to i
 * [rain module install](rain_module_install.md)	 - Install a package of Rain modules from CodeArtifact
 * [rain module publish](rain_module_publish.md)	 - Publish a directory of Rain modules to CodeArtifact
 
-###### Auto generated by spf13/cobra on 31-Oct-2024
+###### Auto generated by spf13/cobra on 8-Nov-2024
diff --git a/docs/rain_module_bootstrap.md b/docs/rain_module_bootstrap.md
index 471c50a3..85dfd3bd 100644
--- a/docs/rain_module_bootstrap.md
+++ b/docs/rain_module_bootstrap.md
@@ -29,4 +29,4 @@ rain module bootstrap [flags]
 
 * [rain module](rain_module.md)	 - Interact with Rain modules in CodeArtifact
 
-###### Auto generated by spf13/cobra on 31-Oct-2024
+###### Auto generated by spf13/cobra on 8-Nov-2024
diff --git a/docs/rain_module_install.md b/docs/rain_module_install.md
index 6ebef1a8..f094d512 100644
--- a/docs/rain_module_install.md
+++ b/docs/rain_module_install.md
@@ -30,4 +30,4 @@ rain module install <name> [flags]
 
 * [rain module](rain_module.md)	 - Interact with Rain modules in CodeArtifact
 
-###### Auto generated by spf13/cobra on 31-Oct-2024
+###### Auto generated by spf13/cobra on 8-Nov-2024
diff --git a/docs/rain_module_publish.md b/docs/rain_module_publish.md
index c2bf1a92..e7214609 100644
--- a/docs/rain_module_publish.md
+++ b/docs/rain_module_publish.md
@@ -30,4 +30,4 @@ rain module publish <name> [flags]
 
 * [rain module](rain_module.md)	 - Interact with Rain modules in CodeArtifact
 
-###### Auto generated by spf13/cobra on 31-Oct-2024
+###### Auto generated by spf13/cobra on 8-Nov-2024
diff --git a/docs/rain_pkg.md b/docs/rain_pkg.md
index 54d88ca4..891e9b99 100644
--- a/docs/rain_pkg.md
+++ b/docs/rain_pkg.md
@@ -69,4 +69,4 @@ rain pkg <template>
 
 * [rain](index.md)	 - 
 
-###### Auto generated by spf13/cobra on 31-Oct-2024
+###### Auto generated by spf13/cobra on 8-Nov-2024
diff --git a/docs/rain_rm.md b/docs/rain_rm.md
index 6ccbb165..8f8dd325 100644
--- a/docs/rain_rm.md
+++ b/docs/rain_rm.md
@@ -34,4 +34,4 @@ rain rm <stack> [changeset]
 
 * [rain](index.md)	 - 
 
-###### Auto generated by spf13/cobra on 31-Oct-2024
+###### Auto generated by spf13/cobra on 8-Nov-2024
diff --git a/docs/rain_stackset.md b/docs/rain_stackset.md
index 6d48acc5..a740d933 100644
--- a/docs/rain_stackset.md
+++ b/docs/rain_stackset.md
@@ -31,4 +31,4 @@ This command manipulates stack sets. It has no action if specific stack set comm
 * [rain stackset ls](rain_stackset_ls.md)	 - List a CloudFormation stack sets in a given region
 * [rain stackset rm](rain_stackset_rm.md)	 - Delete a CloudFormation stack set and/or its instances.
 
-###### Auto generated by spf13/cobra on 31-Oct-2024
+###### Auto generated by spf13/cobra on 8-Nov-2024
diff --git a/docs/rain_stackset_deploy.md b/docs/rain_stackset_deploy.md
index 2e5870ae..9ebc7699 100644
--- a/docs/rain_stackset_deploy.md
+++ b/docs/rain_stackset_deploy.md
@@ -65,4 +65,4 @@ rain stackset deploy <template> [stackset] [flags]
 
 * [rain stackset](rain_stackset.md)	 - This command manipulates stack sets.
 
-###### Auto generated by spf13/cobra on 31-Oct-2024
+###### Auto generated by spf13/cobra on 8-Nov-2024
diff --git a/docs/rain_stackset_ls.md b/docs/rain_stackset_ls.md
index 4cda26cb..56fe9c9a 100644
--- a/docs/rain_stackset_ls.md
+++ b/docs/rain_stackset_ls.md
@@ -31,4 +31,4 @@ rain stackset ls <stack set>
 
 * [rain stackset](rain_stackset.md)	 - This command manipulates stack sets.
 
-###### Auto generated by spf13/cobra on 31-Oct-2024
+###### Auto generated by spf13/cobra on 8-Nov-2024
diff --git a/docs/rain_stackset_rm.md b/docs/rain_stackset_rm.md
index ff2a1325..0b64cafd 100644
--- a/docs/rain_stackset_rm.md
+++ b/docs/rain_stackset_rm.md
@@ -31,4 +31,4 @@ rain stackset rm <stackset>
 
 * [rain stackset](rain_stackset.md)	 - This command manipulates stack sets.
 
-###### Auto generated by spf13/cobra on 31-Oct-2024
+###### Auto generated by spf13/cobra on 8-Nov-2024
diff --git a/docs/rain_tree.md b/docs/rain_tree.md
index 1817f196..89edc506 100644
--- a/docs/rain_tree.md
+++ b/docs/rain_tree.md
@@ -30,4 +30,4 @@ rain tree [template]
 
 * [rain](index.md)	 - 
 
-###### Auto generated by spf13/cobra on 31-Oct-2024
+###### Auto generated by spf13/cobra on 8-Nov-2024
diff --git a/docs/rain_watch.md b/docs/rain_watch.md
index f24b7480..da77f6b6 100644
--- a/docs/rain_watch.md
+++ b/docs/rain_watch.md
@@ -30,4 +30,4 @@ rain watch <stack>
 
 * [rain](index.md)	 - 
 
-###### Auto generated by spf13/cobra on 31-Oct-2024
+###### Auto generated by spf13/cobra on 8-Nov-2024

From dfede6278c452674b656e7997be2362e674f781b Mon Sep 17 00:00:00 2001
From: Eric Beard <ericzbeard@gmail.com>
Date: Mon, 11 Nov 2024 10:13:10 -0800
Subject: [PATCH 14/15] IfNotParam

---
 README.md                          | 24 +++++++++++++
 cft/pkg/ifparams.go                | 58 ++++++++++++++++++++++++++++++
 cft/pkg/module.go                  | 22 ++----------
 cft/pkg/tmpl/ifparam-expect.yaml   | 12 +++++--
 cft/pkg/tmpl/ifparam-module.yaml   | 15 ++++++--
 cft/pkg/tmpl/ifparam-template.yaml | 10 +++---
 docs/README.tmpl                   | 24 +++++++++++++
 docs/index.md                      |  2 +-
 docs/rain_bootstrap.md             |  2 +-
 docs/rain_build.md                 |  2 +-
 docs/rain_cat.md                   |  2 +-
 docs/rain_cc.md                    |  2 +-
 docs/rain_cc_deploy.md             |  2 +-
 docs/rain_cc_drift.md              |  2 +-
 docs/rain_cc_rm.md                 |  2 +-
 docs/rain_cc_state.md              |  2 +-
 docs/rain_console.md               |  2 +-
 docs/rain_deploy.md                |  2 +-
 docs/rain_diff.md                  |  2 +-
 docs/rain_fmt.md                   |  2 +-
 docs/rain_forecast.md              |  2 +-
 docs/rain_info.md                  |  2 +-
 docs/rain_logs.md                  |  2 +-
 docs/rain_ls.md                    |  2 +-
 docs/rain_merge.md                 |  2 +-
 docs/rain_module.md                |  2 +-
 docs/rain_module_bootstrap.md      |  2 +-
 docs/rain_module_install.md        |  2 +-
 docs/rain_module_publish.md        |  2 +-
 docs/rain_pkg.md                   |  2 +-
 docs/rain_rm.md                    |  2 +-
 docs/rain_stackset.md              |  2 +-
 docs/rain_stackset_deploy.md       |  2 +-
 docs/rain_stackset_ls.md           |  2 +-
 docs/rain_stackset_rm.md           |  2 +-
 docs/rain_tree.md                  |  2 +-
 docs/rain_watch.md                 |  2 +-
 37 files changed, 166 insertions(+), 59 deletions(-)
 create mode 100644 cft/pkg/ifparams.go

diff --git a/README.md b/README.md
index 7c70c4d1..b14ea53d 100644
--- a/README.md
+++ b/README.md
@@ -528,6 +528,30 @@ An API Gateway REST API
 
 A Lambda function and associated API Gateway resources
 
+### IfParam and IfNotParam
+
+Inside a module, you can add a Metadata attribute to show or hide resources, 
+depending on whether the parent template sets a parameter value. This is similar 
+to the Conditional section in a template, but somewhat simpler, and it only works in modules.
+
+```yaml
+Resources:
+  Bucket:
+    Type: AWS::S3::Bucket
+    Metadata:
+      Rain:
+        IfParam: Foo
+```
+
+If the parent template does not set a value for the `Foo` property, the module will 
+omit the resource. The opposite is true for `IfNotParam`. 
+
+`IfParam` can be useful to make flexible modules that can optionally do things like 
+configure permissions for related resources, like allowing access to a bucket or table.
+
+`IfNotParam` is useful if you have pre-created a resource and you don't want the module 
+to create it for you.
+
 ### Gantt Chart
 
 Output a chart to an HTML file that you can view with a browser to look at how long stack operations take for each resource.
diff --git a/cft/pkg/ifparams.go b/cft/pkg/ifparams.go
new file mode 100644
index 00000000..b66d274c
--- /dev/null
+++ b/cft/pkg/ifparams.go
@@ -0,0 +1,58 @@
+package pkg
+
+import (
+	"github.com/aws-cloudformation/rain/internal/node"
+	"github.com/aws-cloudformation/rain/internal/s11n"
+	"gopkg.in/yaml.v3"
+)
+
+// omitIfs returns true if the resource should be omitted due to
+// IfParam or IfNotParam, which are simplified Conditionals.
+// IfParam means "if this parameter is set, show this resource"
+// IfNotParam means "if this parameter is empty, show this resource"
+func omitIfs(rainMetadata *yaml.Node, moduleParams *yaml.Node, templateProps *yaml.Node, moduleResource *yaml.Node) bool {
+
+	retval := false
+
+	ifp := s11n.GetValue(rainMetadata, IfParam)
+	// If the value of IfParam is not set, omit this resource
+	if ifp != "" {
+		if moduleParams != nil &&
+			s11n.GetMap(moduleParams, ifp) != nil &&
+			s11n.GetValue(templateProps, ifp) == "" &&
+			len(s11n.GetMap(templateProps, ifp)) == 0 {
+			retval = true
+		}
+		// Get rid of the IfParam, since it's irrelevant in the resulting template
+		node.RemoveFromMap(rainMetadata, IfParam)
+	}
+
+	ifnp := s11n.GetValue(rainMetadata, IfNotParam)
+	// If the value of IfNotParam is set, omit this resource
+	if ifnp != "" {
+		moduleParamExists := s11n.GetMap(moduleParams, ifnp) != nil
+		hasTemplatePropValue := s11n.GetValue(templateProps, ifnp) != ""
+		hasTemplatePropMap := len(s11n.GetMap(templateProps, ifnp)) > 0
+		existsInTemplateProps := hasTemplatePropValue || hasTemplatePropMap
+
+		if moduleParamExists && existsInTemplateProps {
+			retval = true
+		}
+
+		// Get rid of the IfParam, since it's irrelevant in the resulting template
+		node.RemoveFromMap(rainMetadata, IfNotParam)
+	}
+
+	// If the Rain attribute is empty, get rid of it
+	if len(rainMetadata.Content) == 0 {
+		_, metadataNode, _ := s11n.GetMapValue(moduleResource, Metadata)
+		node.RemoveFromMap(metadataNode, Rain)
+
+		// Now if there is nothing left in Metadata, get rid of that too
+		if len(metadataNode.Content) == 0 {
+			node.RemoveFromMap(moduleResource, Metadata)
+		}
+	}
+
+	return retval
+}
diff --git a/cft/pkg/module.go b/cft/pkg/module.go
index fbd8aee2..9bd51465 100644
--- a/cft/pkg/module.go
+++ b/cft/pkg/module.go
@@ -20,6 +20,7 @@ const (
 	Rain                = "Rain"
 	Metadata            = "Metadata"
 	IfParam             = "IfParam"
+	IfNotParam          = "IfNotParam"
 	Overrides           = "Overrides"
 	DependsOn           = "DependsOn"
 	Properties          = "Properties"
@@ -631,25 +632,8 @@ func processModule(
 		metadata := s11n.GetMap(moduleResource, Metadata)
 		if metadata != nil {
 			if rainMetadata, ok := metadata[Rain]; ok {
-				ifp := s11n.GetValue(rainMetadata, IfParam)
-				// If the value of IfParam is not set, omit this resource
-				if ifp != "" {
-					if moduleParams != nil &&
-						s11n.GetMap(moduleParams, ifp) != nil &&
-						s11n.GetValue(templateProps, ifp) == "" &&
-						len(s11n.GetMap(templateProps, ifp)) == 0 {
-						continue
-					} else {
-						// Get rid of the IfParam, since it's irrelevant in the resulting template
-						node.RemoveFromMap(rainMetadata, IfParam)
-						if len(rainMetadata.Content) == 0 {
-							_, metadataNode, _ := s11n.GetMapValue(moduleResource, Metadata)
-							node.RemoveFromMap(metadataNode, Rain)
-							if len(metadataNode.Content) == 0 {
-								node.RemoveFromMap(moduleResource, Metadata)
-							}
-						}
-					}
+				if omitIfs(rainMetadata, moduleParams, templateProps, moduleResource) {
+					continue
 				}
 			}
 		}
diff --git a/cft/pkg/tmpl/ifparam-expect.yaml b/cft/pkg/tmpl/ifparam-expect.yaml
index d63b1fe1..21e19d4c 100644
--- a/cft/pkg/tmpl/ifparam-expect.yaml
+++ b/cft/pkg/tmpl/ifparam-expect.yaml
@@ -1,6 +1,14 @@
 Resources:
-  ShowBucket:
+  Show1Hide2Bucket1:
     Type: AWS::S3::Bucket
-  Show2Bucket:
+    Properties:
+      BucketName: Bucket1
+  Show1Hide2SubBucket1:
     Type: AWS::S3::Bucket
+    Properties:
+      BucketName: Bucket1
+  Hide1Show2Bucket2:
+    Type: AWS::S3::Bucket
+    Properties:
+      BucketName: Bucket2
 
diff --git a/cft/pkg/tmpl/ifparam-module.yaml b/cft/pkg/tmpl/ifparam-module.yaml
index 9a0b21a9..8e37ca14 100644
--- a/cft/pkg/tmpl/ifparam-module.yaml
+++ b/cft/pkg/tmpl/ifparam-module.yaml
@@ -1,11 +1,20 @@
 Parameters:
-  Show:
+  Foo:
     Type: String
 
 Resources:
-  Bucket:
+  Bucket1:
     Type: AWS::S3::Bucket
     Metadata:
       Rain:
-        IfParam: Show
+        IfParam: Foo
+    Properties:
+      BucketName: Bucket1
+  Bucket2:
+    Type: AWS::S3::Bucket
+    Metadata:
+      Rain:
+        IfNotParam: Foo
+    Properties:
+      BucketName: Bucket2
 
diff --git a/cft/pkg/tmpl/ifparam-template.yaml b/cft/pkg/tmpl/ifparam-template.yaml
index 4b97a601..4e2ade6d 100644
--- a/cft/pkg/tmpl/ifparam-template.yaml
+++ b/cft/pkg/tmpl/ifparam-template.yaml
@@ -1,11 +1,11 @@
 Resources:
-  Show:
+  Show1Hide2:
     Type: !Rain::Module ./ifparam-module.yaml
     Properties:
-      Show: true
-  Show2:
+      Foo: foo
+  Show1Hide2Sub:
     Type: !Rain::Module ./ifparam-module.yaml
     Properties:
-      Show: !Sub ${Foo}
-  Hide:
+      Foo: !Sub ${Foo}
+  Hide1Show2:
     Type: !Rain::Module ./ifparam-module.yaml
diff --git a/docs/README.tmpl b/docs/README.tmpl
index 81831c77..4ada1a8a 100644
--- a/docs/README.tmpl
+++ b/docs/README.tmpl
@@ -500,6 +500,30 @@ An API Gateway REST API
 
 A Lambda function and associated API Gateway resources
 
+### IfParam and IfNotParam
+
+Inside a module, you can add a Metadata attribute to show or hide resources, 
+depending on whether the parent template sets a parameter value. This is similar 
+to the Conditional section in a template, but somewhat simpler, and it only works in modules.
+
+```yaml
+Resources:
+  Bucket:
+    Type: AWS::S3::Bucket
+    Metadata:
+      Rain:
+        IfParam: Foo
+```
+
+If the parent template does not set a value for the `Foo` property, the module will 
+omit the resource. The opposite is true for `IfNotParam`. 
+
+`IfParam` can be useful to make flexible modules that can optionally do things like 
+configure permissions for related resources, like allowing access to a bucket or table.
+
+`IfNotParam` is useful if you have pre-created a resource and you don't want the module 
+to create it for you.
+
 ### Gantt Chart
 
 Output a chart to an HTML file that you can view with a browser to look at how long stack operations take for each resource.
diff --git a/docs/index.md b/docs/index.md
index d53fa776..189fe01d 100644
--- a/docs/index.md
+++ b/docs/index.md
@@ -36,4 +36,4 @@ Rain is a command line tool for working with AWS CloudFormation templates and st
 * [rain tree](rain_tree.md)	 - Find dependencies of Resources and Outputs in a local template
 * [rain watch](rain_watch.md)	 - Display an updating view of a CloudFormation stack
 
-###### Auto generated by spf13/cobra on 8-Nov-2024
+###### Auto generated by spf13/cobra on 11-Nov-2024
diff --git a/docs/rain_bootstrap.md b/docs/rain_bootstrap.md
index 522db3b8..e997821f 100644
--- a/docs/rain_bootstrap.md
+++ b/docs/rain_bootstrap.md
@@ -33,4 +33,4 @@ rain bootstrap
 
 * [rain](index.md)	 - 
 
-###### Auto generated by spf13/cobra on 8-Nov-2024
+###### Auto generated by spf13/cobra on 11-Nov-2024
diff --git a/docs/rain_build.md b/docs/rain_build.md
index 9bdec880..4ae01856 100644
--- a/docs/rain_build.md
+++ b/docs/rain_build.md
@@ -41,4 +41,4 @@ rain build [<resource type>] or <prompt>
 
 * [rain](index.md)	 - 
 
-###### Auto generated by spf13/cobra on 8-Nov-2024
+###### Auto generated by spf13/cobra on 11-Nov-2024
diff --git a/docs/rain_cat.md b/docs/rain_cat.md
index a3bf3197..4f769a6b 100644
--- a/docs/rain_cat.md
+++ b/docs/rain_cat.md
@@ -35,4 +35,4 @@ rain cat <stack>
 
 * [rain](index.md)	 - 
 
-###### Auto generated by spf13/cobra on 8-Nov-2024
+###### Auto generated by spf13/cobra on 11-Nov-2024
diff --git a/docs/rain_cc.md b/docs/rain_cc.md
index cbab88ef..4a4eb16a 100644
--- a/docs/rain_cc.md
+++ b/docs/rain_cc.md
@@ -33,4 +33,4 @@ You must pass the --experimental (-x) flag to use this command, to acknowledge t
 * [rain cc rm](rain_cc_rm.md)	 - Delete a deployment created by cc deploy (Experimental!)
 * [rain cc state](rain_cc_state.md)	 - Download the state file for a template deployed with cc deploy
 
-###### Auto generated by spf13/cobra on 8-Nov-2024
+###### Auto generated by spf13/cobra on 11-Nov-2024
diff --git a/docs/rain_cc_deploy.md b/docs/rain_cc_deploy.md
index 7c092307..2601114e 100644
--- a/docs/rain_cc_deploy.md
+++ b/docs/rain_cc_deploy.md
@@ -40,4 +40,4 @@ rain cc deploy <template> <name>
 
 * [rain cc](rain_cc.md)	 - Interact with templates using Cloud Control API instead of CloudFormation
 
-###### Auto generated by spf13/cobra on 8-Nov-2024
+###### Auto generated by spf13/cobra on 11-Nov-2024
diff --git a/docs/rain_cc_drift.md b/docs/rain_cc_drift.md
index f61c8a31..7735c540 100644
--- a/docs/rain_cc_drift.md
+++ b/docs/rain_cc_drift.md
@@ -33,4 +33,4 @@ rain cc drift <name>
 
 * [rain cc](rain_cc.md)	 - Interact with templates using Cloud Control API instead of CloudFormation
 
-###### Auto generated by spf13/cobra on 8-Nov-2024
+###### Auto generated by spf13/cobra on 11-Nov-2024
diff --git a/docs/rain_cc_rm.md b/docs/rain_cc_rm.md
index 677f6a6c..552c0682 100644
--- a/docs/rain_cc_rm.md
+++ b/docs/rain_cc_rm.md
@@ -33,4 +33,4 @@ rain cc rm <name>
 
 * [rain cc](rain_cc.md)	 - Interact with templates using Cloud Control API instead of CloudFormation
 
-###### Auto generated by spf13/cobra on 8-Nov-2024
+###### Auto generated by spf13/cobra on 11-Nov-2024
diff --git a/docs/rain_cc_state.md b/docs/rain_cc_state.md
index 91090f26..111d4f0e 100644
--- a/docs/rain_cc_state.md
+++ b/docs/rain_cc_state.md
@@ -33,4 +33,4 @@ rain cc state <name>
 
 * [rain cc](rain_cc.md)	 - Interact with templates using Cloud Control API instead of CloudFormation
 
-###### Auto generated by spf13/cobra on 8-Nov-2024
+###### Auto generated by spf13/cobra on 11-Nov-2024
diff --git a/docs/rain_console.md b/docs/rain_console.md
index a81062fc..48cd7394 100644
--- a/docs/rain_console.md
+++ b/docs/rain_console.md
@@ -39,4 +39,4 @@ rain console [stack]
 
 * [rain](index.md)	 - 
 
-###### Auto generated by spf13/cobra on 8-Nov-2024
+###### Auto generated by spf13/cobra on 11-Nov-2024
diff --git a/docs/rain_deploy.md b/docs/rain_deploy.md
index b739df38..425d2412 100644
--- a/docs/rain_deploy.md
+++ b/docs/rain_deploy.md
@@ -87,4 +87,4 @@ rain deploy <template> [stack]
 
 * [rain](index.md)	 - 
 
-###### Auto generated by spf13/cobra on 8-Nov-2024
+###### Auto generated by spf13/cobra on 11-Nov-2024
diff --git a/docs/rain_diff.md b/docs/rain_diff.md
index a38c6b91..3a4d6cc0 100644
--- a/docs/rain_diff.md
+++ b/docs/rain_diff.md
@@ -28,4 +28,4 @@ rain diff <from> <to>
 
 * [rain](index.md)	 - 
 
-###### Auto generated by spf13/cobra on 8-Nov-2024
+###### Auto generated by spf13/cobra on 11-Nov-2024
diff --git a/docs/rain_fmt.md b/docs/rain_fmt.md
index b37d2946..8da2fe6e 100644
--- a/docs/rain_fmt.md
+++ b/docs/rain_fmt.md
@@ -37,4 +37,4 @@ rain fmt <filename>...
 
 * [rain](index.md)	 - 
 
-###### Auto generated by spf13/cobra on 8-Nov-2024
+###### Auto generated by spf13/cobra on 11-Nov-2024
diff --git a/docs/rain_forecast.md b/docs/rain_forecast.md
index 7d5a9739..33d5df86 100644
--- a/docs/rain_forecast.md
+++ b/docs/rain_forecast.md
@@ -55,4 +55,4 @@ rain forecast --experimental <template> [stackName]
 
 * [rain](index.md)	 - 
 
-###### Auto generated by spf13/cobra on 8-Nov-2024
+###### Auto generated by spf13/cobra on 11-Nov-2024
diff --git a/docs/rain_info.md b/docs/rain_info.md
index 210ecc51..070a5cad 100644
--- a/docs/rain_info.md
+++ b/docs/rain_info.md
@@ -30,4 +30,4 @@ rain info
 
 * [rain](index.md)	 - 
 
-###### Auto generated by spf13/cobra on 8-Nov-2024
+###### Auto generated by spf13/cobra on 11-Nov-2024
diff --git a/docs/rain_logs.md b/docs/rain_logs.md
index 1504d543..23a8d53c 100644
--- a/docs/rain_logs.md
+++ b/docs/rain_logs.md
@@ -37,4 +37,4 @@ rain logs <stack> (<resource>)
 
 * [rain](index.md)	 - 
 
-###### Auto generated by spf13/cobra on 8-Nov-2024
+###### Auto generated by spf13/cobra on 11-Nov-2024
diff --git a/docs/rain_ls.md b/docs/rain_ls.md
index 37d4dad3..4746dd06 100644
--- a/docs/rain_ls.md
+++ b/docs/rain_ls.md
@@ -31,4 +31,4 @@ rain ls <stack> [changeset]
 
 * [rain](index.md)	 - 
 
-###### Auto generated by spf13/cobra on 8-Nov-2024
+###### Auto generated by spf13/cobra on 11-Nov-2024
diff --git a/docs/rain_merge.md b/docs/rain_merge.md
index 20f3345a..f013c64b 100644
--- a/docs/rain_merge.md
+++ b/docs/rain_merge.md
@@ -30,4 +30,4 @@ rain merge <template> <template> ...
 
 * [rain](index.md)	 - 
 
-###### Auto generated by spf13/cobra on 8-Nov-2024
+###### Auto generated by spf13/cobra on 11-Nov-2024
diff --git a/docs/rain_module.md b/docs/rain_module.md
index 693376b6..845bab03 100644
--- a/docs/rain_module.md
+++ b/docs/rain_module.md
@@ -31,4 +31,4 @@ The rain module command can be used to publish modules to CodeArtifact, and to i
 * [rain module install](rain_module_install.md)	 - Install a package of Rain modules from CodeArtifact
 * [rain module publish](rain_module_publish.md)	 - Publish a directory of Rain modules to CodeArtifact
 
-###### Auto generated by spf13/cobra on 8-Nov-2024
+###### Auto generated by spf13/cobra on 11-Nov-2024
diff --git a/docs/rain_module_bootstrap.md b/docs/rain_module_bootstrap.md
index 85dfd3bd..a862cdda 100644
--- a/docs/rain_module_bootstrap.md
+++ b/docs/rain_module_bootstrap.md
@@ -29,4 +29,4 @@ rain module bootstrap [flags]
 
 * [rain module](rain_module.md)	 - Interact with Rain modules in CodeArtifact
 
-###### Auto generated by spf13/cobra on 8-Nov-2024
+###### Auto generated by spf13/cobra on 11-Nov-2024
diff --git a/docs/rain_module_install.md b/docs/rain_module_install.md
index f094d512..724828c2 100644
--- a/docs/rain_module_install.md
+++ b/docs/rain_module_install.md
@@ -30,4 +30,4 @@ rain module install <name> [flags]
 
 * [rain module](rain_module.md)	 - Interact with Rain modules in CodeArtifact
 
-###### Auto generated by spf13/cobra on 8-Nov-2024
+###### Auto generated by spf13/cobra on 11-Nov-2024
diff --git a/docs/rain_module_publish.md b/docs/rain_module_publish.md
index e7214609..73dc5b32 100644
--- a/docs/rain_module_publish.md
+++ b/docs/rain_module_publish.md
@@ -30,4 +30,4 @@ rain module publish <name> [flags]
 
 * [rain module](rain_module.md)	 - Interact with Rain modules in CodeArtifact
 
-###### Auto generated by spf13/cobra on 8-Nov-2024
+###### Auto generated by spf13/cobra on 11-Nov-2024
diff --git a/docs/rain_pkg.md b/docs/rain_pkg.md
index 891e9b99..96ea3688 100644
--- a/docs/rain_pkg.md
+++ b/docs/rain_pkg.md
@@ -69,4 +69,4 @@ rain pkg <template>
 
 * [rain](index.md)	 - 
 
-###### Auto generated by spf13/cobra on 8-Nov-2024
+###### Auto generated by spf13/cobra on 11-Nov-2024
diff --git a/docs/rain_rm.md b/docs/rain_rm.md
index 8f8dd325..b25fc58b 100644
--- a/docs/rain_rm.md
+++ b/docs/rain_rm.md
@@ -34,4 +34,4 @@ rain rm <stack> [changeset]
 
 * [rain](index.md)	 - 
 
-###### Auto generated by spf13/cobra on 8-Nov-2024
+###### Auto generated by spf13/cobra on 11-Nov-2024
diff --git a/docs/rain_stackset.md b/docs/rain_stackset.md
index a740d933..c34230dd 100644
--- a/docs/rain_stackset.md
+++ b/docs/rain_stackset.md
@@ -31,4 +31,4 @@ This command manipulates stack sets. It has no action if specific stack set comm
 * [rain stackset ls](rain_stackset_ls.md)	 - List a CloudFormation stack sets in a given region
 * [rain stackset rm](rain_stackset_rm.md)	 - Delete a CloudFormation stack set and/or its instances.
 
-###### Auto generated by spf13/cobra on 8-Nov-2024
+###### Auto generated by spf13/cobra on 11-Nov-2024
diff --git a/docs/rain_stackset_deploy.md b/docs/rain_stackset_deploy.md
index 9ebc7699..dc13a0b1 100644
--- a/docs/rain_stackset_deploy.md
+++ b/docs/rain_stackset_deploy.md
@@ -65,4 +65,4 @@ rain stackset deploy <template> [stackset] [flags]
 
 * [rain stackset](rain_stackset.md)	 - This command manipulates stack sets.
 
-###### Auto generated by spf13/cobra on 8-Nov-2024
+###### Auto generated by spf13/cobra on 11-Nov-2024
diff --git a/docs/rain_stackset_ls.md b/docs/rain_stackset_ls.md
index 56fe9c9a..422923f3 100644
--- a/docs/rain_stackset_ls.md
+++ b/docs/rain_stackset_ls.md
@@ -31,4 +31,4 @@ rain stackset ls <stack set>
 
 * [rain stackset](rain_stackset.md)	 - This command manipulates stack sets.
 
-###### Auto generated by spf13/cobra on 8-Nov-2024
+###### Auto generated by spf13/cobra on 11-Nov-2024
diff --git a/docs/rain_stackset_rm.md b/docs/rain_stackset_rm.md
index 0b64cafd..2eae3739 100644
--- a/docs/rain_stackset_rm.md
+++ b/docs/rain_stackset_rm.md
@@ -31,4 +31,4 @@ rain stackset rm <stackset>
 
 * [rain stackset](rain_stackset.md)	 - This command manipulates stack sets.
 
-###### Auto generated by spf13/cobra on 8-Nov-2024
+###### Auto generated by spf13/cobra on 11-Nov-2024
diff --git a/docs/rain_tree.md b/docs/rain_tree.md
index 89edc506..cf6ef0dd 100644
--- a/docs/rain_tree.md
+++ b/docs/rain_tree.md
@@ -30,4 +30,4 @@ rain tree [template]
 
 * [rain](index.md)	 - 
 
-###### Auto generated by spf13/cobra on 8-Nov-2024
+###### Auto generated by spf13/cobra on 11-Nov-2024
diff --git a/docs/rain_watch.md b/docs/rain_watch.md
index da77f6b6..bb92ea06 100644
--- a/docs/rain_watch.md
+++ b/docs/rain_watch.md
@@ -30,4 +30,4 @@ rain watch <stack>
 
 * [rain](index.md)	 - 
 
-###### Auto generated by spf13/cobra on 8-Nov-2024
+###### Auto generated by spf13/cobra on 11-Nov-2024

From 88743ec00d64b82e3eba983a40df7339d9d498be Mon Sep 17 00:00:00 2001
From: Eric Beard <ericzbeard@gmail.com>
Date: Mon, 11 Nov 2024 10:24:26 -0800
Subject: [PATCH 15/15] regen recommended templates

---
 test/templates/build/bucket/bucket.yaml       |  62 ++++----
 test/templates/build/bucket/website.yaml      | 118 +++++++--------
 test/templates/build/ecs/cluster.yaml         | 107 +++++++-------
 .../build/eventbridge/central-logs.yaml       |  10 +-
 test/templates/build/pipeline/codecommit.yaml |   4 +-
 test/templates/build/pipeline/s3.yaml         |   6 +-
 test/templates/build/vpc/vpc.yaml             |  52 +++----
 test/templates/build/webapp/webapp.yaml       | 134 +++++++++---------
 8 files changed, 246 insertions(+), 247 deletions(-)

diff --git a/test/templates/build/bucket/bucket.yaml b/test/templates/build/bucket/bucket.yaml
index c32b7d62..17b61496 100644
--- a/test/templates/build/bucket/bucket.yaml
+++ b/test/templates/build/bucket/bucket.yaml
@@ -8,17 +8,30 @@ Description: |
 
 Parameters:
   AppName:
-    Type: String
     Description: This name is used as a prefix for resource names
+    Type: String
 
   Content:
-    Type: String
     Description: An optional directory to upload to the bucket
+    Type: String
     Default: RAIN_NO_CONTENT
 
 Resources:
   ObjectStorageLogBucket:
     Type: AWS::S3::Bucket
+    Metadata:
+      Comment: This bucket records access logs for the main bucket
+      checkov:
+        skip:
+          - comment: This is the log bucket
+            id: CKV_AWS_18
+      guard:
+        SuppressedRules:
+          - S3_BUCKET_LOGGING_ENABLED
+          - S3_BUCKET_REPLICATION_ENABLED
+      Rain:
+        Content: RAIN_NO_CONTENT
+        EmptyOnDelete: false
     Properties:
       BucketEncryption:
         ServerSideEncryptionConfiguration:
@@ -39,22 +52,16 @@ Resources:
         RestrictPublicBuckets: true
       VersioningConfiguration:
         Status: Enabled
+
+  ObjectStorageBucket:
+    Type: AWS::S3::Bucket
     Metadata:
-      Comment: This bucket records access logs for the main bucket
-      checkov:
-        skip:
-          - comment: This is the log bucket
-            id: CKV_AWS_18
       guard:
         SuppressedRules:
-          - S3_BUCKET_LOGGING_ENABLED
-          - S3_BUCKET_REPLICATION_ENABLED
+          - S3_BUCKET_DEFAULT_LOCK_ENABLED
       Rain:
-        Content: RAIN_NO_CONTENT
+        Content: !Ref Content
         EmptyOnDelete: false
-
-  ObjectStorageBucket:
-    Type: AWS::S3::Bucket
     Properties:
       BucketEncryption:
         ServerSideEncryptionConfiguration:
@@ -77,16 +84,23 @@ Resources:
             Status: Enabled
       VersioningConfiguration:
         Status: Enabled
+
+  ObjectStorageReplicaBucket:
+    Type: AWS::S3::Bucket
     Metadata:
+      Comment: This bucket is used as a target for replicas from the main bucket
+      checkov:
+        skip:
+          - comment: This is the replica bucket
+            id: CKV_AWS_18
       guard:
         SuppressedRules:
           - S3_BUCKET_DEFAULT_LOCK_ENABLED
+          - S3_BUCKET_REPLICATION_ENABLED
+          - S3_BUCKET_LOGGING_ENABLED
       Rain:
-        Content: !Ref Content
+        Content: RAIN_NO_CONTENT
         EmptyOnDelete: false
-
-  ObjectStorageReplicaBucket:
-    Type: AWS::S3::Bucket
     Properties:
       BucketEncryption:
         ServerSideEncryptionConfiguration:
@@ -101,20 +115,6 @@ Resources:
         RestrictPublicBuckets: true
       VersioningConfiguration:
         Status: Enabled
-    Metadata:
-      Comment: This bucket is used as a target for replicas from the main bucket
-      checkov:
-        skip:
-          - comment: This is the replica bucket
-            id: CKV_AWS_18
-      guard:
-        SuppressedRules:
-          - S3_BUCKET_DEFAULT_LOCK_ENABLED
-          - S3_BUCKET_REPLICATION_ENABLED
-          - S3_BUCKET_LOGGING_ENABLED
-      Rain:
-        Content: RAIN_NO_CONTENT
-        EmptyOnDelete: false
 
   ObjectStorageReplicationPolicy:
     Type: AWS::IAM::RolePolicy
diff --git a/test/templates/build/bucket/website.yaml b/test/templates/build/bucket/website.yaml
index 52c7332a..e7cf4adc 100644
--- a/test/templates/build/bucket/website.yaml
+++ b/test/templates/build/bucket/website.yaml
@@ -8,16 +8,16 @@ Description: |
 
 Parameters:
   AppName:
-    Type: String
     Description: This name be used as a prefix for resource names
+    Type: String
 
   WebACL:
-    Type: String
     Description: The web acl id of a CLOUDFRONT scoped web acl in us-east-1
+    Type: String
 
   Content:
-    Type: String
     Description: An optional directory to upload to the bucket
+    Type: String
     Default: RAIN_NO_CONTENT
 
 Resources:
@@ -74,6 +74,19 @@ Resources:
 
   ContentLogBucket:
     Type: AWS::S3::Bucket
+    Metadata:
+      Comment: This bucket records access logs for the main bucket
+      checkov:
+        skip:
+          - comment: This is the log bucket
+            id: CKV_AWS_18
+      guard:
+        SuppressedRules:
+          - S3_BUCKET_LOGGING_ENABLED
+          - S3_BUCKET_REPLICATION_ENABLED
+      Rain:
+        Content: RAIN_NO_CONTENT
+        EmptyOnDelete: false
     Properties:
       BucketEncryption:
         ServerSideEncryptionConfiguration:
@@ -94,22 +107,16 @@ Resources:
         RestrictPublicBuckets: true
       VersioningConfiguration:
         Status: Enabled
+
+  ContentBucket:
+    Type: AWS::S3::Bucket
     Metadata:
-      Comment: This bucket records access logs for the main bucket
-      checkov:
-        skip:
-          - comment: This is the log bucket
-            id: CKV_AWS_18
       guard:
         SuppressedRules:
-          - S3_BUCKET_LOGGING_ENABLED
-          - S3_BUCKET_REPLICATION_ENABLED
+          - S3_BUCKET_DEFAULT_LOCK_ENABLED
       Rain:
-        Content: RAIN_NO_CONTENT
+        Content: !Ref Content
         EmptyOnDelete: false
-
-  ContentBucket:
-    Type: AWS::S3::Bucket
     Properties:
       BucketEncryption:
         ServerSideEncryptionConfiguration:
@@ -132,16 +139,23 @@ Resources:
             Status: Enabled
       VersioningConfiguration:
         Status: Enabled
+
+  ContentReplicaBucket:
+    Type: AWS::S3::Bucket
     Metadata:
+      Comment: This bucket is used as a target for replicas from the main bucket
+      checkov:
+        skip:
+          - comment: This is the replica bucket
+            id: CKV_AWS_18
       guard:
         SuppressedRules:
           - S3_BUCKET_DEFAULT_LOCK_ENABLED
+          - S3_BUCKET_REPLICATION_ENABLED
+          - S3_BUCKET_LOGGING_ENABLED
       Rain:
-        Content: !Ref Content
+        Content: RAIN_NO_CONTENT
         EmptyOnDelete: false
-
-  ContentReplicaBucket:
-    Type: AWS::S3::Bucket
     Properties:
       BucketEncryption:
         ServerSideEncryptionConfiguration:
@@ -156,20 +170,6 @@ Resources:
         RestrictPublicBuckets: true
       VersioningConfiguration:
         Status: Enabled
-    Metadata:
-      Comment: This bucket is used as a target for replicas from the main bucket
-      checkov:
-        skip:
-          - comment: This is the replica bucket
-            id: CKV_AWS_18
-      guard:
-        SuppressedRules:
-          - S3_BUCKET_DEFAULT_LOCK_ENABLED
-          - S3_BUCKET_REPLICATION_ENABLED
-          - S3_BUCKET_LOGGING_ENABLED
-      Rain:
-        Content: RAIN_NO_CONTENT
-        EmptyOnDelete: false
 
   ContentReplicationPolicy:
     Type: AWS::IAM::RolePolicy
@@ -300,6 +300,19 @@ Resources:
 
   CloudFrontLogsLogBucket:
     Type: AWS::S3::Bucket
+    Metadata:
+      Comment: This bucket records access logs for the main bucket
+      checkov:
+        skip:
+          - comment: This is the log bucket
+            id: CKV_AWS_18
+      guard:
+        SuppressedRules:
+          - S3_BUCKET_LOGGING_ENABLED
+          - S3_BUCKET_REPLICATION_ENABLED
+      Rain:
+        Content: RAIN_NO_CONTENT
+        EmptyOnDelete: false
     Properties:
       BucketEncryption:
         ServerSideEncryptionConfiguration:
@@ -320,22 +333,16 @@ Resources:
         RestrictPublicBuckets: true
       VersioningConfiguration:
         Status: Enabled
+
+  CloudFrontLogsBucket:
+    Type: AWS::S3::Bucket
     Metadata:
-      Comment: This bucket records access logs for the main bucket
-      checkov:
-        skip:
-          - comment: This is the log bucket
-            id: CKV_AWS_18
       guard:
         SuppressedRules:
-          - S3_BUCKET_LOGGING_ENABLED
-          - S3_BUCKET_REPLICATION_ENABLED
+          - S3_BUCKET_DEFAULT_LOCK_ENABLED
       Rain:
         Content: RAIN_NO_CONTENT
         EmptyOnDelete: false
-
-  CloudFrontLogsBucket:
-    Type: AWS::S3::Bucket
     Properties:
       BucketEncryption:
         ServerSideEncryptionConfiguration:
@@ -358,16 +365,23 @@ Resources:
             Status: Enabled
       VersioningConfiguration:
         Status: Enabled
+
+  CloudFrontLogsReplicaBucket:
+    Type: AWS::S3::Bucket
     Metadata:
+      Comment: This bucket is used as a target for replicas from the main bucket
+      checkov:
+        skip:
+          - comment: This is the replica bucket
+            id: CKV_AWS_18
       guard:
         SuppressedRules:
           - S3_BUCKET_DEFAULT_LOCK_ENABLED
+          - S3_BUCKET_REPLICATION_ENABLED
+          - S3_BUCKET_LOGGING_ENABLED
       Rain:
         Content: RAIN_NO_CONTENT
         EmptyOnDelete: false
-
-  CloudFrontLogsReplicaBucket:
-    Type: AWS::S3::Bucket
     Properties:
       BucketEncryption:
         ServerSideEncryptionConfiguration:
@@ -382,20 +396,6 @@ Resources:
         RestrictPublicBuckets: true
       VersioningConfiguration:
         Status: Enabled
-    Metadata:
-      Comment: This bucket is used as a target for replicas from the main bucket
-      checkov:
-        skip:
-          - comment: This is the replica bucket
-            id: CKV_AWS_18
-      guard:
-        SuppressedRules:
-          - S3_BUCKET_DEFAULT_LOCK_ENABLED
-          - S3_BUCKET_REPLICATION_ENABLED
-          - S3_BUCKET_LOGGING_ENABLED
-      Rain:
-        Content: RAIN_NO_CONTENT
-        EmptyOnDelete: false
 
   CloudFrontLogsReplicationPolicy:
     Type: AWS::IAM::RolePolicy
diff --git a/test/templates/build/ecs/cluster.yaml b/test/templates/build/ecs/cluster.yaml
index 493d357f..ad169cda 100644
--- a/test/templates/build/ecs/cluster.yaml
+++ b/test/templates/build/ecs/cluster.yaml
@@ -3,16 +3,16 @@ Description: |
 
 Parameters:
   ImageURL:
-    Type: String
     Description: The URL to the container image
+    Type: String
 
   CertificateArn:
-    Type: String
     Description: The arn to the certificate for the load balancer
+    Type: String
 
   AppName:
-    Type: String
     Description: This will be used as part of resource names
+    Type: String
 
 Resources:
   Cluster:
@@ -101,6 +101,8 @@ Resources:
         - !Ref TaskExecutionRole
 
   LogGroup:
+    DeletionPolicy: Delete
+    UpdateReplacePolicy: Delete
     Type: AWS::Logs::LogGroup
     Metadata:
       checkov:
@@ -112,11 +114,12 @@ Resources:
     Properties:
       LogGroupName: !Sub ${AppName}-svc-logs
       RetentionInDays: 30
-    UpdateReplacePolicy: Delete
-    DeletionPolicy: Delete
 
   Service:
     Type: AWS::ECS::Service
+    DependsOn:
+      - ELBLoadBalancerListener
+      - TaskRole
     Metadata:
       cfn-lint:
         config:
@@ -148,12 +151,11 @@ Resources:
             - !Ref NetworkPrivateSubnet1Subnet
             - !Ref NetworkPrivateSubnet2Subnet
       TaskDefinition: !Ref TaskDefinition
-    DependsOn:
-      - ELBLoadBalancerListener
-      - TaskRole
 
   ServiceSecurityGroup:
     Type: AWS::EC2::SecurityGroup
+    DependsOn:
+      - TaskRole
     Properties:
       GroupDescription: !Sub ${AppName}-svc-service/SecurityGroup
       SecurityGroupEgress:
@@ -161,11 +163,10 @@ Resources:
           Description: Allow all outbound traffic by default
           IpProtocol: "-1"
       VpcId: !Ref NetworkVPC
-    DependsOn:
-      - TaskRole
 
   Ingress:
     Type: AWS::EC2::SecurityGroupIngress
+    DependsOn: TaskRole
     Properties:
       Description: Load balancer to target
       FromPort: 80
@@ -173,7 +174,6 @@ Resources:
       IpProtocol: tcp
       SourceSecurityGroupId: !GetAtt ELBLoadBalancerSecurityGroup.GroupId
       ToPort: 80
-    DependsOn: TaskRole
 
   NetworkVPC:
     Type: AWS::EC2::VPC
@@ -185,6 +185,10 @@ Resources:
 
   NetworkPublicSubnet1:
     Type: AWS::EC2::Subnet
+    Metadata:
+      guard:
+        SuppressedRules:
+          - SUBNET_AUTO_ASSIGN_PUBLIC_IP_DISABLED
     Properties:
       AvailabilityZone: !Select
         - 0
@@ -193,10 +197,6 @@ Resources:
       CidrBlock: 10.0.0.0/18
       MapPublicIpOnLaunch: true
       VpcId: !Ref NetworkVPC
-    Metadata:
-      guard:
-        SuppressedRules:
-          - SUBNET_AUTO_ASSIGN_PUBLIC_IP_DISABLED
 
   NetworkPublicSubnet1RouteTable:
     Type: AWS::EC2::RouteTable
@@ -211,16 +211,16 @@ Resources:
 
   NetworkPublicSubnet1DefaultRoute:
     Type: AWS::EC2::Route
-    Properties:
-      DestinationCidrBlock: 0.0.0.0/0
-      GatewayId: !Ref NetworkInternetGateway
-      RouteTableId: !Ref NetworkPublicSubnet1RouteTable
+    DependsOn:
+      - NetworkVPCGW
     Metadata:
       guard:
         SuppressedRules:
           - NO_UNRESTRICTED_ROUTE_TO_IGW
-    DependsOn:
-      - NetworkVPCGW
+    Properties:
+      DestinationCidrBlock: 0.0.0.0/0
+      GatewayId: !Ref NetworkInternetGateway
+      RouteTableId: !Ref NetworkPublicSubnet1RouteTable
 
   NetworkPublicSubnet1EIP:
     Type: AWS::EC2::EIP
@@ -229,15 +229,19 @@ Resources:
 
   NetworkPublicSubnet1NATGateway:
     Type: AWS::EC2::NatGateway
-    Properties:
-      AllocationId: !GetAtt NetworkPublicSubnet1EIP.AllocationId
-      SubnetId: !Ref NetworkPublicSubnet1
     DependsOn:
       - NetworkPublicSubnet1DefaultRoute
       - NetworkPublicSubnet1RouteTableAssociation
+    Properties:
+      AllocationId: !GetAtt NetworkPublicSubnet1EIP.AllocationId
+      SubnetId: !Ref NetworkPublicSubnet1
 
   NetworkPublicSubnet2:
     Type: AWS::EC2::Subnet
+    Metadata:
+      guard:
+        SuppressedRules:
+          - SUBNET_AUTO_ASSIGN_PUBLIC_IP_DISABLED
     Properties:
       AvailabilityZone: !Select
         - 1
@@ -246,10 +250,6 @@ Resources:
       CidrBlock: 10.0.64.0/18
       MapPublicIpOnLaunch: true
       VpcId: !Ref NetworkVPC
-    Metadata:
-      guard:
-        SuppressedRules:
-          - SUBNET_AUTO_ASSIGN_PUBLIC_IP_DISABLED
 
   NetworkPublicSubnet2RouteTable:
     Type: AWS::EC2::RouteTable
@@ -264,16 +264,16 @@ Resources:
 
   NetworkPublicSubnet2DefaultRoute:
     Type: AWS::EC2::Route
-    Properties:
-      DestinationCidrBlock: 0.0.0.0/0
-      GatewayId: !Ref NetworkInternetGateway
-      RouteTableId: !Ref NetworkPublicSubnet2RouteTable
+    DependsOn:
+      - NetworkVPCGW
     Metadata:
       guard:
         SuppressedRules:
           - NO_UNRESTRICTED_ROUTE_TO_IGW
-    DependsOn:
-      - NetworkVPCGW
+    Properties:
+      DestinationCidrBlock: 0.0.0.0/0
+      GatewayId: !Ref NetworkInternetGateway
+      RouteTableId: !Ref NetworkPublicSubnet2RouteTable
 
   NetworkPublicSubnet2EIP:
     Type: AWS::EC2::EIP
@@ -282,12 +282,12 @@ Resources:
 
   NetworkPublicSubnet2NATGateway:
     Type: AWS::EC2::NatGateway
-    Properties:
-      AllocationId: !GetAtt NetworkPublicSubnet2EIP.AllocationId
-      SubnetId: !Ref NetworkPublicSubnet2
     DependsOn:
       - NetworkPublicSubnet2DefaultRoute
       - NetworkPublicSubnet2RouteTableAssociation
+    Properties:
+      AllocationId: !GetAtt NetworkPublicSubnet2EIP.AllocationId
+      SubnetId: !Ref NetworkPublicSubnet2
 
   NetworkPrivateSubnet1Subnet:
     Type: AWS::EC2::Subnet
@@ -358,6 +358,18 @@ Resources:
 
   ELBLoadBalancer:
     Type: AWS::ElasticLoadBalancingV2::LoadBalancer
+    DependsOn:
+      - NetworkPublicSubnet1DefaultRoute
+      - NetworkPublicSubnet1RouteTableAssociation
+      - NetworkPublicSubnet2DefaultRoute
+      - NetworkPublicSubnet2RouteTableAssociation
+    Metadata:
+      checkov:
+        skip:
+          - id: CKV_AWS_91
+      guard:
+        SuppressedRules:
+          - ELB_DELETION_PROTECTION_ENABLED
     Properties:
       LoadBalancerAttributes:
         - Key: deletion_protection.enabled
@@ -379,19 +391,6 @@ Resources:
       #  - PublicSubnet1RouteTableAssociation
       #  - PublicSubnet2DefaultRoute
       #  - PublicSubnet2RouteTableAssociation
-    Metadata:
-      checkov:
-        skip:
-          - id: CKV_AWS_91
-      guard:
-        SuppressedRules:
-          - ELB_DELETION_PROTECTION_ENABLED
-    DependsOn:
-      - NetworkPublicSubnet1DefaultRoute
-      - NetworkPublicSubnet1RouteTableAssociation
-      - NetworkPublicSubnet2DefaultRoute
-      - NetworkPublicSubnet2RouteTableAssociation
-
   ELBLoadBalancerSecurityGroup:
     Type: AWS::EC2::SecurityGroup
     Properties:
@@ -416,6 +415,10 @@ Resources:
 
   ELBLoadBalancerListener:
     Type: AWS::ElasticLoadBalancingV2::Listener
+    Metadata:
+      guard:
+        SuppressedRules:
+          - ELBV2_ACM_CERTIFICATE_REQUIRED
     Properties:
       DefaultActions:
         - TargetGroupArn: !Ref ELBTargetGroup
@@ -426,10 +429,6 @@ Resources:
       Certificates:
         - CertificateArn: !Ref CertificateArn
       SslPolicy: ELBSecurityPolicy-TLS13-1-2-2021-06
-    Metadata:
-      guard:
-        SuppressedRules:
-          - ELBV2_ACM_CERTIFICATE_REQUIRED
 
   ELBTargetGroup:
     Type: AWS::ElasticLoadBalancingV2::TargetGroup
diff --git a/test/templates/build/eventbridge/central-logs.yaml b/test/templates/build/eventbridge/central-logs.yaml
index e793e740..a4cb49bc 100644
--- a/test/templates/build/eventbridge/central-logs.yaml
+++ b/test/templates/build/eventbridge/central-logs.yaml
@@ -1,17 +1,15 @@
 AWSTemplateFormatVersion: "2010-09-09"
 
-Transform: AWS::LanguageExtensions
-
 Description: "A central event bus rule and log group to collect CloudFormation logs \nfrom all target accounts. This example can be adapted to collect logs\nfrom other services besides CloudFormation, by editing the embedded template \nthat is used to create stack instances in target accounts.\n"
 
 Parameters:
   OUID:
-    Type: String
     Description: "The Id of the Organization Unit to deploy the stack set to. \nAll accounts under this OU will receive stack instances of the \nchild template embedded in this template.\n"
+    Type: String
 
   OrgID:
-    Type: String
     Description: "The Id of the Organization to verify the cross account API call. \nAll accounts in this org will be granted permissions to put events \nonto the default event bus in this account. \nNote that this is not the OUID, it's the org itself and should start with \"o-\"\n"
+    Type: String
 
   CentralEventBusName:
     Type: String
@@ -21,6 +19,8 @@ Parameters:
     Type: String
     Default: central-cloudformation-logs
 
+Transform: AWS::LanguageExtensions
+
 Resources:
   CentralEventBus:
     Type: AWS::Events::EventBus
@@ -47,8 +47,8 @@ Resources:
             aws:PrincipalOrgID: !Ref OrgID
 
   CentralEventLog:
-    DependsOn: CentralEventBus
     Type: AWS::Logs::LogGroup
+    DependsOn: CentralEventBus
     Properties:
       LogGroupClass: STANDARD
       LogGroupName: !Ref CentralEventLogName
diff --git a/test/templates/build/pipeline/codecommit.yaml b/test/templates/build/pipeline/codecommit.yaml
index 9d9b677d..604b8a9a 100644
--- a/test/templates/build/pipeline/codecommit.yaml
+++ b/test/templates/build/pipeline/codecommit.yaml
@@ -8,12 +8,12 @@ Description: |
 
 Parameters:
   ArtifactBucket:
-    Type: String
     Description: The name of the bucket to use for storing pipeline artifacts
+    Type: String
 
   AppName:
-    Type: String
     Description: This will be used for resource names
+    Type: String
 
   Branch:
     Description: The name of the source branch in CodeCommit
diff --git a/test/templates/build/pipeline/s3.yaml b/test/templates/build/pipeline/s3.yaml
index d1cd6850..b815d918 100644
--- a/test/templates/build/pipeline/s3.yaml
+++ b/test/templates/build/pipeline/s3.yaml
@@ -7,16 +7,16 @@ Description: |
 
 Parameters:
   SourceBucket:
-    Type: String
     Description: The name of the bucket to use for source code
+    Type: String
 
   ArtifactBucket:
-    Type: String
     Description: The name of the bucket to use for storing pipeline artifacts
+    Type: String
 
   AppName:
-    Type: String
     Description: This will be used for resource names
+    Type: String
 
 Resources:
   Pipeline:
diff --git a/test/templates/build/vpc/vpc.yaml b/test/templates/build/vpc/vpc.yaml
index 520d2311..793b7e87 100644
--- a/test/templates/build/vpc/vpc.yaml
+++ b/test/templates/build/vpc/vpc.yaml
@@ -64,6 +64,10 @@ Resources:
 
   NetworkPublicSubnet1:
     Type: AWS::EC2::Subnet
+    Metadata:
+      guard:
+        SuppressedRules:
+          - SUBNET_AUTO_ASSIGN_PUBLIC_IP_DISABLED
     Properties:
       AvailabilityZone: !Select
         - 0
@@ -72,10 +76,6 @@ Resources:
       CidrBlock: 10.0.0.0/18
       MapPublicIpOnLaunch: true
       VpcId: !Ref NetworkVPC
-    Metadata:
-      guard:
-        SuppressedRules:
-          - SUBNET_AUTO_ASSIGN_PUBLIC_IP_DISABLED
 
   NetworkPublicSubnet1RouteTable:
     Type: AWS::EC2::RouteTable
@@ -90,16 +90,16 @@ Resources:
 
   NetworkPublicSubnet1DefaultRoute:
     Type: AWS::EC2::Route
-    Properties:
-      DestinationCidrBlock: 0.0.0.0/0
-      GatewayId: !Ref NetworkInternetGateway
-      RouteTableId: !Ref NetworkPublicSubnet1RouteTable
+    DependsOn:
+      - NetworkVPCGW
     Metadata:
       guard:
         SuppressedRules:
           - NO_UNRESTRICTED_ROUTE_TO_IGW
-    DependsOn:
-      - NetworkVPCGW
+    Properties:
+      DestinationCidrBlock: 0.0.0.0/0
+      GatewayId: !Ref NetworkInternetGateway
+      RouteTableId: !Ref NetworkPublicSubnet1RouteTable
 
   NetworkPublicSubnet1EIP:
     Type: AWS::EC2::EIP
@@ -108,15 +108,19 @@ Resources:
 
   NetworkPublicSubnet1NATGateway:
     Type: AWS::EC2::NatGateway
-    Properties:
-      AllocationId: !GetAtt NetworkPublicSubnet1EIP.AllocationId
-      SubnetId: !Ref NetworkPublicSubnet1
     DependsOn:
       - NetworkPublicSubnet1DefaultRoute
       - NetworkPublicSubnet1RouteTableAssociation
+    Properties:
+      AllocationId: !GetAtt NetworkPublicSubnet1EIP.AllocationId
+      SubnetId: !Ref NetworkPublicSubnet1
 
   NetworkPublicSubnet2:
     Type: AWS::EC2::Subnet
+    Metadata:
+      guard:
+        SuppressedRules:
+          - SUBNET_AUTO_ASSIGN_PUBLIC_IP_DISABLED
     Properties:
       AvailabilityZone: !Select
         - 1
@@ -125,10 +129,6 @@ Resources:
       CidrBlock: 10.0.64.0/18
       MapPublicIpOnLaunch: true
       VpcId: !Ref NetworkVPC
-    Metadata:
-      guard:
-        SuppressedRules:
-          - SUBNET_AUTO_ASSIGN_PUBLIC_IP_DISABLED
 
   NetworkPublicSubnet2RouteTable:
     Type: AWS::EC2::RouteTable
@@ -143,16 +143,16 @@ Resources:
 
   NetworkPublicSubnet2DefaultRoute:
     Type: AWS::EC2::Route
-    Properties:
-      DestinationCidrBlock: 0.0.0.0/0
-      GatewayId: !Ref NetworkInternetGateway
-      RouteTableId: !Ref NetworkPublicSubnet2RouteTable
+    DependsOn:
+      - NetworkVPCGW
     Metadata:
       guard:
         SuppressedRules:
           - NO_UNRESTRICTED_ROUTE_TO_IGW
-    DependsOn:
-      - NetworkVPCGW
+    Properties:
+      DestinationCidrBlock: 0.0.0.0/0
+      GatewayId: !Ref NetworkInternetGateway
+      RouteTableId: !Ref NetworkPublicSubnet2RouteTable
 
   NetworkPublicSubnet2EIP:
     Type: AWS::EC2::EIP
@@ -161,12 +161,12 @@ Resources:
 
   NetworkPublicSubnet2NATGateway:
     Type: AWS::EC2::NatGateway
-    Properties:
-      AllocationId: !GetAtt NetworkPublicSubnet2EIP.AllocationId
-      SubnetId: !Ref NetworkPublicSubnet2
     DependsOn:
       - NetworkPublicSubnet2DefaultRoute
       - NetworkPublicSubnet2RouteTableAssociation
+    Properties:
+      AllocationId: !GetAtt NetworkPublicSubnet2EIP.AllocationId
+      SubnetId: !Ref NetworkPublicSubnet2
 
   NetworkPrivateSubnet1Subnet:
     Type: AWS::EC2::Subnet
diff --git a/test/templates/build/webapp/webapp.yaml b/test/templates/build/webapp/webapp.yaml
index 5abdfada..3736d871 100644
--- a/test/templates/build/webapp/webapp.yaml
+++ b/test/templates/build/webapp/webapp.yaml
@@ -2,18 +2,18 @@ Description: Creates a web application with a static website using S3 and CloudF
 
 Parameters:
   AppName:
-    Type: String
     Description: This name is used as a prefix for resource names
+    Type: String
     Default: rain-webapp-sample
 
   LambdaCodeS3Bucket:
-    Type: String
     Description: The bucket where your lambda handler is
+    Type: String
     Default: rain-artifacts-207567786752-us-east-1
 
   LambdaCodeS3Key:
-    Type: String
     Description: The object key for your lambda handler
+    Type: String
     Default: 512113b95e9fc6345b2e19a43350af82aaa815011120288f16b1f281d5efdc95
 
 Resources:
@@ -76,6 +76,16 @@ Resources:
 
   SiteDistribution:
     Type: AWS::CloudFront::Distribution
+    Metadata:
+      checkov:
+        skip:
+          - id: CKV_AWS_174
+            comment: Using the default cloudfront certificate with no aliases
+      guard:
+        SuppressedRules:
+          - CLOUDFRONT_CUSTOM_SSL_CERTIFICATE
+          - CLOUDFRONT_ORIGIN_FAILOVER_ENABLED
+          - CLOUDFRONT_SNI_ENABLED
     Properties:
       DistributionConfig:
         DefaultCacheBehavior:
@@ -98,16 +108,6 @@ Resources:
         ViewerCertificate:
           CloudFrontDefaultCertificate: true
         WebACLId: !GetAtt SiteWebACL.Arn
-    Metadata:
-      checkov:
-        skip:
-          - id: CKV_AWS_174
-            comment: Using the default cloudfront certificate with no aliases
-      guard:
-        SuppressedRules:
-          - CLOUDFRONT_CUSTOM_SSL_CERTIFICATE
-          - CLOUDFRONT_ORIGIN_FAILOVER_ENABLED
-          - CLOUDFRONT_SNI_ENABLED
 
   SiteWebACL:
     Type: AWS::WAFv2::WebACL
@@ -154,6 +154,16 @@ Resources:
 
   SiteContentLogBucket:
     Type: AWS::S3::Bucket
+    Metadata:
+      Comment: This bucket records access logs for the main bucket
+      checkov:
+        skip:
+          - comment: This is the log bucket
+            id: CKV_AWS_18
+      guard:
+        SuppressedRules:
+          - S3_BUCKET_LOGGING_ENABLED
+          - S3_BUCKET_REPLICATION_ENABLED
     Properties:
       BucketEncryption:
         ServerSideEncryptionConfiguration:
@@ -174,19 +184,13 @@ Resources:
         RestrictPublicBuckets: true
       VersioningConfiguration:
         Status: Enabled
-    Metadata:
-      Comment: This bucket records access logs for the main bucket
-      checkov:
-        skip:
-          - comment: This is the log bucket
-            id: CKV_AWS_18
-      guard:
-        SuppressedRules:
-          - S3_BUCKET_LOGGING_ENABLED
-          - S3_BUCKET_REPLICATION_ENABLED
 
   SiteContentBucket:
     Type: AWS::S3::Bucket
+    Metadata:
+      guard:
+        SuppressedRules:
+          - S3_BUCKET_DEFAULT_LOCK_ENABLED
     Properties:
       BucketEncryption:
         ServerSideEncryptionConfiguration:
@@ -209,13 +213,20 @@ Resources:
             Status: Enabled
       VersioningConfiguration:
         Status: Enabled
+
+  SiteContentReplicaBucket:
+    Type: AWS::S3::Bucket
     Metadata:
+      Comment: This bucket is used as a target for replicas from the main bucket
+      checkov:
+        skip:
+          - comment: This is the replica bucket
+            id: CKV_AWS_18
       guard:
         SuppressedRules:
           - S3_BUCKET_DEFAULT_LOCK_ENABLED
-
-  SiteContentReplicaBucket:
-    Type: AWS::S3::Bucket
+          - S3_BUCKET_REPLICATION_ENABLED
+          - S3_BUCKET_LOGGING_ENABLED
     Properties:
       BucketEncryption:
         ServerSideEncryptionConfiguration:
@@ -230,17 +241,6 @@ Resources:
         RestrictPublicBuckets: true
       VersioningConfiguration:
         Status: Enabled
-    Metadata:
-      Comment: This bucket is used as a target for replicas from the main bucket
-      checkov:
-        skip:
-          - comment: This is the replica bucket
-            id: CKV_AWS_18
-      guard:
-        SuppressedRules:
-          - S3_BUCKET_DEFAULT_LOCK_ENABLED
-          - S3_BUCKET_REPLICATION_ENABLED
-          - S3_BUCKET_LOGGING_ENABLED
 
   SiteContentReplicationPolicy:
     Type: AWS::IAM::RolePolicy
@@ -379,6 +379,16 @@ Resources:
 
   SiteCloudFrontLogsLogBucket:
     Type: AWS::S3::Bucket
+    Metadata:
+      Comment: This bucket records access logs for the main bucket
+      checkov:
+        skip:
+          - comment: This is the log bucket
+            id: CKV_AWS_18
+      guard:
+        SuppressedRules:
+          - S3_BUCKET_LOGGING_ENABLED
+          - S3_BUCKET_REPLICATION_ENABLED
     Properties:
       BucketEncryption:
         ServerSideEncryptionConfiguration:
@@ -399,19 +409,13 @@ Resources:
         RestrictPublicBuckets: true
       VersioningConfiguration:
         Status: Enabled
-    Metadata:
-      Comment: This bucket records access logs for the main bucket
-      checkov:
-        skip:
-          - comment: This is the log bucket
-            id: CKV_AWS_18
-      guard:
-        SuppressedRules:
-          - S3_BUCKET_LOGGING_ENABLED
-          - S3_BUCKET_REPLICATION_ENABLED
 
   SiteCloudFrontLogsBucket:
     Type: AWS::S3::Bucket
+    Metadata:
+      guard:
+        SuppressedRules:
+          - S3_BUCKET_DEFAULT_LOCK_ENABLED
     Properties:
       BucketEncryption:
         ServerSideEncryptionConfiguration:
@@ -437,13 +441,20 @@ Resources:
       OwnershipControls:
         Rules:
           - ObjectOwnership: BucketOwnerPreferred
+
+  SiteCloudFrontLogsReplicaBucket:
+    Type: AWS::S3::Bucket
     Metadata:
+      Comment: This bucket is used as a target for replicas from the main bucket
+      checkov:
+        skip:
+          - comment: This is the replica bucket
+            id: CKV_AWS_18
       guard:
         SuppressedRules:
           - S3_BUCKET_DEFAULT_LOCK_ENABLED
-
-  SiteCloudFrontLogsReplicaBucket:
-    Type: AWS::S3::Bucket
+          - S3_BUCKET_REPLICATION_ENABLED
+          - S3_BUCKET_LOGGING_ENABLED
     Properties:
       BucketEncryption:
         ServerSideEncryptionConfiguration:
@@ -458,17 +469,6 @@ Resources:
         RestrictPublicBuckets: true
       VersioningConfiguration:
         Status: Enabled
-    Metadata:
-      Comment: This bucket is used as a target for replicas from the main bucket
-      checkov:
-        skip:
-          - comment: This is the replica bucket
-            id: CKV_AWS_18
-      guard:
-        SuppressedRules:
-          - S3_BUCKET_DEFAULT_LOCK_ENABLED
-          - S3_BUCKET_REPLICATION_ENABLED
-          - S3_BUCKET_LOGGING_ENABLED
 
   SiteCloudFrontLogsReplicationPolicy:
     Type: AWS::IAM::RolePolicy
@@ -599,6 +599,8 @@ Resources:
 
   CognitoUserPool:
     Type: AWS::Cognito::UserPool
+    DependsOn:
+      - SiteDistribution
     Properties:
       UserPoolName: !Ref AppName
       AdminCreateUserConfig:
@@ -612,8 +614,6 @@ Resources:
           Required: true
         - Name: family_name
           Required: true
-    DependsOn:
-      - SiteDistribution
 
   CognitoDomain:
     Type: AWS::Cognito::UserPoolDomain
@@ -833,15 +833,15 @@ Resources:
 
   RestApiDeployment:
     Type: AWS::ApiGateway::Deployment
-    Properties:
-      RestApiId: !Ref RestApi
-    Metadata:
-      Version: 2
     DependsOn:
       - TestResourceGet
       - TestResourceOptions
       - JwtResourceGet
       - JwtResourceOptions
+    Metadata:
+      Version: 2
+    Properties:
+      RestApiId: !Ref RestApi
 
   RestApiStage:
     Type: AWS::ApiGateway::Stage