Merge pull request #675 from ericzbeard/awscli-modules

Add S3 URI handling to modules

Author: ericzbeard

cft/pkg/conditions.go

@@ -124,7 +124,7 @@ func evalCond(condName string, condValue interface{}, conditions map[string]inte
 			return evaluateNot(not, conditions, module)
 		}
 		if equals, ok := v["Fn::Equals"]; ok {
-			return evaluateEquals(equals, module)
+			return evaluateEquals(equals)
 		}
 		if condition, ok := v["Condition"]; ok {
 			// Reference to another condition
@@ -210,7 +210,7 @@ func evaluateNot(notExpr interface{}, conditions map[string]interface{}, module
 }
 
 // evaluateEquals evaluates an Fn::Equals condition
-func evaluateEquals(equalsExpr interface{}, module *Module) (bool, error) {
+func evaluateEquals(equalsExpr interface{}) (bool, error) {
 	equalsList, ok := equalsExpr.([]interface{})
 	if !ok || len(equalsList) != 2 {
 		return false, fmt.Errorf("Fn::Equals requires exactly two values")

cft/pkg/content.go

@@ -17,33 +17,21 @@ type ModuleContent struct {
 	BaseUri    string
 }
 
-// Helper function to handle zip file extraction with proper path resolution
-func handleZipFile(root string, location string, hash string, path string) ([]byte, error) {
-	// Resolve the path to the zip file if it's a local path
-	zipPath := location
-	if !strings.HasPrefix(zipPath, "http://") && !strings.HasPrefix(zipPath, "https://") {
-		// If it's a relative path, resolve it relative to the template's directory
-		if !filepath.IsAbs(zipPath) {
-			zipPath = filepath.Join(root, zipPath)
-		}
-	}
+func isHttpsUrl(uri string) bool {
+	return strings.HasPrefix(uri, "https://")
+}
 
-	// Check if the zip file exists if it's a local file
-	if !strings.HasPrefix(zipPath, "http://") && !strings.HasPrefix(zipPath, "https://") {
-		_, err := os.Stat(zipPath)
-		if err != nil {
-			return nil, fmt.Errorf("error accessing zip file %s: %v", zipPath, err)
-		}
-	}
+func isS3URI(uri string) bool {
+	return strings.HasPrefix(uri, "s3://")
+}
 
-	// Unzip, verify hash if there is one, and put the files in memory
-	content, err := DownloadFromZip(zipPath, hash, path)
-	if err != nil {
-		config.Debugf("ZIP: Error extracting from zip: %v", err)
-		return nil, err
+// resolveZipLocation ensures that local zip paths are resolved relative to the template's directory
+func resolveZipLocation(root string, zipLocation string) string {
+	// For local files, resolve the path relative to the template's directory
+	if !isS3URI(zipLocation) && !isHttpsUrl(zipLocation) && !filepath.IsAbs(zipLocation) {
+		return filepath.Join(root, zipLocation)
 	}
-
-	return content, nil
+	return zipLocation
 }
 
 // Get the module's content from a local file, memory, or a remote uri
@@ -54,6 +42,8 @@ func getModuleContent(
 	baseUri string,
 	uri string) (*ModuleContent, error) {
 
+	config.Debugf("getModuleContent root: %s, uri: %s", root, uri)
+
 	var content []byte
 	var err error
 	var newRootDir string
@@ -71,7 +61,8 @@ func getModuleContent(
 
 					if strings.HasSuffix(packageAlias.Location, ".zip") {
 						isZip = true
-						content, err = handleZipFile(root, packageAlias.Location, packageAlias.Hash, path)
+						zipLocation := resolveZipLocation(root, packageAlias.Location)
+						content, err = DownloadFromZip(zipLocation, packageAlias.Hash, path)
 						if err != nil {
 							return nil, err
 						}
@@ -92,12 +83,21 @@ func getModuleContent(
 	// getModuleContent: root=cft/pkg/tmpl/awscli-modules, baseUri=, uri=package.zip/zip-module.yaml
 	if strings.Contains(uri, ".zip/") {
 		isZip = true
-		tokens := strings.Split(uri, "/")
-		location := tokens[0]
-		path := strings.Join(tokens[1:], "/")
-		content, err = handleZipFile(root, location, "", path)
-		if err != nil {
-			return nil, err
+		
+		// Extract the zip location and path within the zip
+		zipIndex := strings.Index(uri, ".zip/")
+		if zipIndex > 0 {
+			zipLocation := uri[:zipIndex+4]  // Include the .zip part
+			zipPath := uri[zipIndex+5:]      // Skip the .zip/ part
+			
+			zipLocation = resolveZipLocation(root, zipLocation)
+			config.Debugf("Extracting from zip: %s, path: %s", zipLocation, zipPath)
+			
+			// Use DownloadFromZip directly - it can handle S3, HTTPS, and local files
+			content, err = DownloadFromZip(zipLocation, "", zipPath)
+			if err != nil {
+				return nil, err
+			}
 		}
 	}
 
@@ -109,7 +109,8 @@ func getModuleContent(
 			path := strings.Replace(uri, packageAlias.Alias+"/", "", 1)
 			if strings.HasSuffix(packageAlias.Location, ".zip") {
 				isZip = true
-				content, err = handleZipFile(root, packageAlias.Location, packageAlias.Hash, path)
+				zipLocation := resolveZipLocation(root, packageAlias.Location)
+				content, err = DownloadFromZip(zipLocation, packageAlias.Hash, path)
 				if err != nil {
 					return nil, err
 				}
@@ -122,7 +123,7 @@ func getModuleContent(
 	// Is this a local file or a URL or did we already unzip a package?
 	if isZip {
 		config.Debugf("Using content from a zipped module package (length: %d bytes)", len(content))
-	} else if strings.HasPrefix(uri, "https://") {
+	} else if isHttpsUrl(uri) || isS3URI(uri) {
 		config.Debugf("Downloading from URL: %s", uri)
 		content, err = downloadModule(uri)
 		if err != nil {
@@ -138,6 +139,7 @@ func getModuleContent(
 		baseUri = strings.Join(urlParts[:len(urlParts)-1], "/")
 
 	} else {
+		config.Debugf("Downloading from a local file, baseUri=%s, uri=%s", baseUri, uri)
 		if baseUri != "" {
 			// If we have a base URL, prepend it to the relative path
 			uri = baseUri + "/" + uri

cft/pkg/download.go

@@ -11,6 +11,7 @@ import (
 	"path/filepath"
 	"strings"
 
+	"github.com/aws-cloudformation/rain/internal/aws/s3"
 	"github.com/aws-cloudformation/rain/internal/config"
 	"github.com/google/uuid"
 )
@@ -42,11 +43,25 @@ func downloadHash(uri string) (string, error) {
 
 // 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("DownloadFromZip uriString: %s, path: %s",
+		uriString, path)
+
 	var zipData []byte
 	var err error
-	
-	// Check if it's a URL or local file
-	if strings.HasPrefix(uriString, "http://") || strings.HasPrefix(uriString, "https://") {
+
+	isUrl := isHttpsUrl(uriString)
+	isS3 := isS3URI(uriString)
+
+	// Check if it's an S3 URI, HTTPS URL, or local file
+	if isS3 {
+		// Download from S3
+		config.Debugf("Downloading from S3: %s", uriString)
+		zipData, err = downloadS3(uriString)
+		if err != nil {
+			return nil, fmt.Errorf("failed to download zip from S3: %v", err)
+		}
+	} else if isUrl {
 		// Download from URL
 		config.Debugf("Downloading %s", uriString)
 		resp, err := http.Get(uriString)
@@ -59,7 +74,7 @@ func DownloadFromZip(uriString string, verifyHash string, path string) ([]byte,
 				config.Debugf("Error closing body: %v", err)
 			}
 		}(resp.Body)
-		
+
 		zipData, err = io.ReadAll(resp.Body)
 		if err != nil {
 			return nil, err
@@ -112,7 +127,7 @@ func DownloadFromZip(uriString string, verifyHash string, path string) ([]byte,
 
 		// Download or read the hash
 		var originalHash string
-		if strings.HasPrefix(verifyHash, "http://") || strings.HasPrefix(verifyHash, "https://") {
+		if isUrl {
 			originalHash, err = downloadHash(verifyHash)
 			if err != nil {
 				return nil, err
@@ -215,9 +230,31 @@ func Unzip(f *os.File, dest string) error {
 	return nil
 }
 
+func downloadS3(uri string) ([]byte, error) {
+	// Parse the S3 URI
+	bucket, key, err := s3.ParseURI(uri)
+	if err != nil {
+		return nil, err
+	}
+
+	// Download the file from S3
+	content, err := s3.GetObject(bucket, key)
+	if err != nil {
+		return nil, err
+	}
+
+	return content, 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)
+
+	// If it's an S3 uri, use the s3 package to download the file
+	if strings.HasPrefix(uri, "s3://") {
+		return downloadS3(uri)
+	}
+
 	resp, err := http.Get(uri)
 	if err != nil {
 		return nil, err

cft/pkg/module.go

@@ -128,11 +128,9 @@ func processModulesSection(t *cft.Template, n *yaml.Node,
 		outputNode := node.MakeMapping()
 
 		err = processModule(
-			name,
 			parsed,
 			outputNode,
 			t,
-			parsed.AsTemplate.Constants,
 			moduleConfig,
 			parentModule)
 		if err != nil {
@@ -151,9 +149,6 @@ func processModulesSection(t *cft.Template, n *yaml.Node,
 			config.Debugf("processModuleSection %s outputNode did not have any Resources", name)
 		}
 
-		config.Debugf("\nparent template after %s processModule ==== \n%s\n",
-			name, node.YamlStr(t.Node))
-
 	}
 
 	// Look for GetAtts like Content[].Arn that reference
@@ -188,11 +183,9 @@ func addScalarAttribute(out *yaml.Node, name string, moduleResource *yaml.Node,
 
 // processModule performs all of the module logic and injects the content into the parent
 func processModule(
-	logicalId string,
 	parsedModule *ParsedModule,
 	outputNode *yaml.Node,
 	t *cft.Template,
-	moduleConstants map[string]*yaml.Node,
 	moduleConfig *cft.ModuleConfig,
 	parentModule *Module) error {
 
@@ -223,8 +216,7 @@ func processModule(
 		},
 	}
 
-	err = processRainSection(moduleAsTemplate,
-		parsedModule.RootDir, parsedModule.FS)
+	err = processRainSection(moduleAsTemplate)
 	if err != nil {
 		return err
 	}
@@ -251,9 +243,6 @@ func processModule(
 		return err
 	}
 
-	config.Debugf("Module %s about to resolve t.Node in processModule",
-		m.Config.Name)
-
 	// Resolve any references to this module in the parent template
 	//err = m.Resolve(t.Node)
 	//if err != nil {
@@ -376,8 +365,6 @@ func (module *Module) ProcessResources(outputNode *yaml.Node) error {
 		// Some refs are to other resources in the module
 		// Other refs are to the module's parameters
 
-		config.Debugf("%s about to resolve resource %s", module.Config.Name, nameNode.Value)
-
 		err = module.Resolve(clonedResource)
 		if err != nil {
 			return fmt.Errorf("failed to resolve refs: %v", err)
@@ -403,13 +390,6 @@ func (module *Module) ProcessResources(outputNode *yaml.Node) error {
 		// if we try to resolve it again later.
 		module.ParentTemplate.AddResolvedModuleNode(clonedResource)
 
-		parentName := ""
-		if module.ParentModule != nil {
-			parentName = module.ParentModule.Config.Name
-		}
-		config.Debugf("Module %s (p:%s) adding resource:\n%s\n", module.Config.Name,
-			parentName,
-			node.YamlStr(clonedResource))
 	}
 
 	return nil
@@ -422,7 +402,6 @@ func processRainResourceModule(
 	outputNode *yaml.Node,
 	t *cft.Template,
 	parent node.NodePair,
-	moduleConstants map[string]*yaml.Node,
 	source string,
 	parsed *ParsedModule) error {
 
@@ -455,7 +434,7 @@ func processRainResourceModule(
 	}
 	moduleConfig.Source = source
 
-	return processModule(logicalId, parsed, outputNode, t, moduleConstants, moduleConfig, nil)
+	return processModule(parsed, outputNode, t, moduleConfig, nil)
 }
 
 func checkPackageAlias(t *cft.Template, uri string) *cft.PackageAlias {
@@ -551,7 +530,7 @@ func module(ctx *directiveContext) (bool, error) {
 	}
 
 	// This needs to happen before recursing, since sub-modules need resolved constants in the parent
-	err = processRainSection(moduleAsTemplate, moduleContent.NewRootDir, ctx.fs)
+	err = processRainSection(moduleAsTemplate)
 	if err != nil {
 		return false, err
 	}
@@ -570,8 +549,7 @@ func module(ctx *directiveContext) (bool, error) {
 
 	// Create a new node to represent the parsed module
 	var outputNode yaml.Node
-	err = processRainResourceModule(moduleNode,
-		&outputNode, t, parent, moduleAsTemplate.Constants, uri, parsed)
+	err = processRainResourceModule(moduleNode, &outputNode, t, parent, uri, parsed)
 	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)

cft/pkg/pkg.go

@@ -106,7 +106,7 @@ func transform(ctx *transformContext) (bool, error) {
 
 // processRainSection returns true if the Rain section of the template existed
 // The section is removed by this function
-func processRainSection(t *cft.Template, rootDir string, fs *embed.FS) error {
+func processRainSection(t *cft.Template) error {
 	t.Constants = make(map[string]*yaml.Node)
 	rainNode, err := t.GetSection(cft.Rain)
 	if err != nil {
@@ -139,7 +139,7 @@ func Template(t *cft.Template, rootDir string, fs *embed.FS) (*cft.Template, err
 	var err error
 
 	// First look for a Rain section and store constants
-	err = processRainSection(t, rootDir, fs)
+	err = processRainSection(t)
 	if err != nil {
 		return nil, fmt.Errorf("failed to process Rain section: %v", err)
 	}

cft/pkg/resolve.go

@@ -24,13 +24,8 @@ func (module *Module) Resolve(n *yaml.Node) error {
 	vf := func(v *visitor.Visitor) {
 		vn := v.GetYamlNode()
 
-		if vn.Value != "" {
-			config.Debugf("Resolve %s", vn.Value)
-		}
-
 		if vn == module.Config.Node {
 			// Don't resolve my own config
-			config.Debugf("Resolve skipping self:\n%s\n", node.YamlStr(vn))
 			v.SkipChildren()
 			return
 		}
@@ -44,7 +39,6 @@ func (module *Module) Resolve(n *yaml.Node) error {
 		if module.ParentTemplate.ModuleAlreadyResolved(vn) {
 			// If we marked a node as resolved, skip all
 			// of its child nodes
-			config.Debugf("Resolve skipping:\n%s\n", node.YamlStr(vn))
 			v.SkipChildren()
 			return
 		}
@@ -182,8 +176,6 @@ func (module *Module) resolveParam(params *yaml.Node, n *yaml.Node, parentProps
 // ${Foo.Bar} is treated like a GetAtt.
 func (module *Module) ResolveSub(n *yaml.Node) error {
 
-	original := node.Clone(n)
-
 	prop := n.Content[1]
 	words, err := parse.ParseSub(prop.Value, true)
 	if err != nil {
@@ -256,10 +248,6 @@ func (module *Module) ResolveSub(n *yaml.Node) error {
 
 	*n = *newProp
 
-	config.Debugf("ResolveSub:\n%s\n -> \n%s\n",
-		node.YamlStr(original),
-		node.YamlStr(n))
-
 	return nil
 }