Repository: klothoplatform/klotho
Pull request #640 (2023-05-25 21:22:10)

static units (no CDN or Cloudfront yet)
In service of #607 

`ts-static-unit` succeeds, as long as you don't ask for a CDN.

> **Note**
> Relative to the version before node expansion (8d98439), this version removes the payloads bucket (`aws:s3:Bucket` `ystest--nternal-lotho-ayloads` when I ran it). Please verify that that's correct.

### Standard checks

- **Unit tests**: added
- **Docs**: n/a
- **Backwards compatibility**: no issues



Patch:
diff --git a/pkg/collectionutil/maps.go b/pkg/collectionutil/maps.go
new file mode 100644
index 00000000..d0318782
--- /dev/null
+++ b/pkg/collectionutil/maps.go
@@ -0,0 +1,20 @@
+package collectionutil
+
+func Keys[K comparable, V any](m map[K]V) []K {
+	keys := make([]K, 0, len(m))
+	for k := range m {
+		keys = append(keys, k)
+	}
+	return keys
+}
+
+// GetOneEntry gets an entry from the given map. If the map contains multiple entries, it's undefined which this
+// returns. If the map is empty, this will return the zero value for both the key and map.
+func GetOneEntry[K comparable, V any](m map[K]V) (K, V) {
+	for k, v := range m {
+		return k, v
+	}
+	var k K
+	var v V
+	return k, v
+}
diff --git a/pkg/provider/aws/fs.go b/pkg/provider/aws/fs.go
index a4c447c8..2f838804 100644
--- a/pkg/provider/aws/fs.go
+++ b/pkg/provider/aws/fs.go
@@ -17,7 +17,7 @@ func (a *AWS) expandFs(dag *core.ResourceGraph, fs *core.Fs) error {
 	return a.MapResourceToConstruct(bucket, fs)
 }
 
-func (a *AWS) getFsConfiguration() resources.S3BucketConfigureParams {
+func getFsConfiguration() resources.S3BucketConfigureParams {
 	return resources.S3BucketConfigureParams{
 		ForceDestroy: true,
 	}
diff --git a/pkg/provider/aws/knowledgebase/edges.go b/pkg/provider/aws/knowledgebase/edges.go
index 60ebf09b..a6396b9e 100644
--- a/pkg/provider/aws/knowledgebase/edges.go
+++ b/pkg/provider/aws/knowledgebase/edges.go
@@ -10,7 +10,7 @@ import (
 
 func GetAwsKnowledgeBase() (knowledgebase.EdgeKB, error) {
 	var err error
-	kbsToUse := []knowledgebase.EdgeKB{AwsExtraEdgesKB, IamKB, NetworkingKB, RdsKB, LambdaKB, ApiGatewayKB, EksKB, ElasticacheKB, LbKB}
+	kbsToUse := []knowledgebase.EdgeKB{AwsExtraEdgesKB, IamKB, NetworkingKB, RdsKB, LambdaKB, ApiGatewayKB, EksKB, ElasticacheKB, LbKB, S3KB}
 	awsKB := make(knowledgebase.EdgeKB)
 	for _, kb := range kbsToUse {
 		for edge, detail := range kb {
diff --git a/pkg/provider/aws/knowledgebase/s3.go b/pkg/provider/aws/knowledgebase/s3.go
new file mode 100644
index 00000000..a6284fc4
--- /dev/null
+++ b/pkg/provider/aws/knowledgebase/s3.go
@@ -0,0 +1,10 @@
+package knowledgebase
+
+import (
+	knowledgebase "github.com/klothoplatform/klotho/pkg/knowledge_base"
+	"github.com/klothoplatform/klotho/pkg/provider/aws/resources"
+)
+
+var S3KB = knowledgebase.Build(
+	knowledgebase.EdgeBuilder[*resources.S3Object, *resources.S3Bucket]{},
+)
diff --git a/pkg/provider/aws/plugin.go b/pkg/provider/aws/plugin.go
index 6305f414..2e56c326 100644
--- a/pkg/provider/aws/plugin.go
+++ b/pkg/provider/aws/plugin.go
@@ -1,6 +1,10 @@
 package aws
 
 import (
+	"sort"
+	"strings"
+
+	"github.com/klothoplatform/klotho/pkg/collectionutil"
 	"github.com/klothoplatform/klotho/pkg/core"
 	"github.com/klothoplatform/klotho/pkg/infra/kubernetes"
 	knowledgebase "github.com/klothoplatform/klotho/pkg/knowledge_base"
@@ -29,6 +33,8 @@ func (a *AWS) ExpandConstructs(result *core.ConstructGraph, dag *core.ResourceGr
 			merr.Append(a.expandKv(dag, construct))
 		case *core.RedisNode:
 			merr.Append(a.expandRedisNode(dag, construct))
+		case *core.StaticUnit:
+			merr.Append(a.expandStaticUnit(dag, construct))
 		}
 	}
 	return merr.ErrOrNil()
@@ -140,13 +146,49 @@ func (a *AWS) configureResources(result *core.ConstructGraph, dag *core.Resource
 				continue
 			}
 		case *resources.S3Bucket:
-			configuration = a.getFsConfiguration()
+			configuration, err = getS3BucketConfig(res, result)
+			if err != nil {
+				merr.Append(err)
+				continue
+			}
 		}
 		merr.Append(dag.CallConfigure(resource, configuration))
 	}
 	return merr.ErrOrNil()
 }
 
+func getS3BucketConfig(bucket *resources.S3Bucket, constructs *core.ConstructGraph) (resources.S3BucketConfigureParams, error) {
+	staticUnits := make(map[string]*core.StaticUnit)
+	for consRef := range bucket.ConstructsRef {
+		cons := constructs.GetConstruct(consRef.ToId())
+		if oneUnit, isUnit := cons.(*core.StaticUnit); isUnit {
+			staticUnits[oneUnit.ID] = oneUnit
+		}
+	}
+	switch len(staticUnits) {
+	case 0:
+		// None of the bucket's constructs were static unit; assume it's an FS
+		return getFsConfiguration(), nil
+	case 1:
+		// The bucket came from a single static unit; gets its params
+		_, unit := collectionutil.GetOneEntry(staticUnits)
+		params := resources.S3BucketConfigureParams{
+			ForceDestroy:  true,
+			IndexDocument: unit.IndexDocument,
+		}
+		return params, nil
+	default:
+		// The bucket came from multiple static units; this is an error
+		ids := collectionutil.Keys(staticUnits)
+		sort.Strings(ids)
+		return resources.S3BucketConfigureParams{}, errors.Errorf(
+			`couldn't resolve configuration for bucket "%s" because I found multiple static units for it: %s`,
+			bucket.Id().String(),
+			strings.Join(ids, ","),
+		)
+	}
+}
+
 func (a *AWS) Translate(result *core.ConstructGraph, dag *core.ResourceGraph) (links []core.CloudResourceLink, err error) {
 	err = a.ExpandConstructs(result, dag)
 	if err != nil {
diff --git a/pkg/provider/aws/resources/s3.go b/pkg/provider/aws/resources/s3.go
index 686e77fa..2d9acc4d 100644
--- a/pkg/provider/aws/resources/s3.go
+++ b/pkg/provider/aws/resources/s3.go
@@ -80,8 +80,9 @@ func (bucket *S3Bucket) Create(dag *core.ResourceGraph, params S3BucketCreatePar
 		if !ok {
 			return errors.Errorf(`found an existing element at %s, but it was not an S3Bucket`, bucket.Id().String())
 		}
-		// Multiple resources may create the same bucket (today, this specifically happens with our payload bucket). If
-		// that happens, just append the refs and exit early; the rest would have been idempotent.
+		// Multiple resources may create the same bucket (for example, this happens with our payload bucket and with
+		// static unit S3Objects). If that happens, just append the refs and exit early; the rest would have been
+		// idempotent.
 		existingS3.ConstructsRef.AddAll(params.Refs)
 		return nil
 	}
@@ -102,14 +103,30 @@ func (bucket *S3Bucket) Configure(params S3BucketConfigureParams) error {
 	return nil
 }
 
-func NewS3Object(bucket *S3Bucket, objectName string, key string, path string) *S3Object {
-	return &S3Object{
-		Name:          objectSanitizer.Apply(fmt.Sprintf("%s-%s", bucket.Name, objectName)),
-		ConstructsRef: bucket.KlothoConstructRef(),
-		Key:           key,
-		FilePath:      path,
-		Bucket:        bucket,
+type S3ObjectCreateParams struct {
+	AppName    string
+	Refs       core.AnnotationKeySet
+	BucketName string
+	Name       string
+	Key        string
+	FilePath   string
+}
+
+func (object *S3Object) Create(dag *core.ResourceGraph, params S3ObjectCreateParams) error {
+	object.Name = objectSanitizer.Apply(fmt.Sprintf("%s-%s-%s", params.AppName, params.BucketName, params.Name))
+	if dag.GetResource(object.Id()) != nil {
+		return fmt.Errorf(`S3Object with name %s already exists`, object.Name)
 	}
+	object.ConstructsRef = params.Refs
+	object.Key = params.Key
+	object.FilePath = params.FilePath
+	return dag.CreateDependencies(object, map[string]any{
+		"Bucket": S3BucketCreateParams{
+			AppName: params.AppName,
+			Refs:    params.Refs,
+			Name:    params.BucketName,
+		},
+	})
 }
 
 // KlothoConstructRef returns AnnotationKey of the klotho resource the cloud resource is correlated to
diff --git a/pkg/provider/aws/static_unit.go b/pkg/provider/aws/static_unit.go
index 5c1c1c5c..ba0381cc 100644
--- a/pkg/provider/aws/static_unit.go
+++ b/pkg/provider/aws/static_unit.go
@@ -3,21 +3,42 @@ package aws
 import (
 	"path/filepath"
 
+	"github.com/klothoplatform/klotho/pkg/collectionutil"
 	"github.com/klothoplatform/klotho/pkg/core"
+	"github.com/klothoplatform/klotho/pkg/multierr"
 	"github.com/klothoplatform/klotho/pkg/provider/aws/resources"
+	"github.com/pkg/errors"
+	"go.uber.org/zap"
 )
 
-func (a *AWS) GenerateStaticUnitResources(unit *core.StaticUnit, dag *core.ResourceGraph) error {
-
-	bucket := resources.NewS3Bucket(unit, a.Config.AppName)
-	bucket.IndexDocument = unit.IndexDocument
-	dag.AddResource(bucket)
+func (a *AWS) expandStaticUnit(dag *core.ResourceGraph, unit *core.StaticUnit) error {
+	errs := multierr.Error{}
+	createdBuckets := make(map[core.ResourceId]*resources.S3Bucket)
 	for _, f := range unit.Files() {
-		object := resources.NewS3Object(bucket, filepath.Base(f.Path()), f.Path(), filepath.Join(unit.ID, f.Path()))
-		dag.AddResource(object)
-		dag.AddDependency(object, bucket)
-		a.MapResourceDirectlyToConstruct(object, unit)
+		object, err := core.CreateResource[*resources.S3Object](dag, resources.S3ObjectCreateParams{
+			AppName:    a.Config.AppName,
+			Refs:       core.AnnotationKeySetOf(unit.AnnotationKey),
+			BucketName: unit.Provenance().ID,
+			Name:       filepath.Base(f.Path()),
+			Key:        f.Path(),
+			FilePath:   filepath.Join(unit.ID, f.Path()),
+		})
+		if err != nil {
+			errs.Append(err)
+			continue
+		}
+		createdBuckets[object.Bucket.Id()] = object.Bucket
+	}
+	nBuckets := len(createdBuckets)
+	zap.L().With()
+	if nBuckets > 0 {
+		if nBuckets > 1 {
+			errs.Append(errors.Errorf(`Found too many buckets for unit %s. This is an internal error.`, unit.Id()))
+		}
+		_, bucket := collectionutil.GetOneEntry(createdBuckets)
+		err := a.MapResourceToConstruct(bucket, unit)
+		errs.Append(err)
 	}
-	a.MapResourceDirectlyToConstruct(bucket, unit)
-	return nil
+
+	return errs.ErrOrNil()
 }


Test patch:
diff --git a/pkg/core/coretesting/expandable_resource.go b/pkg/core/coretesting/expandable_resource.go
index ed2ec7ac..ac266b90 100644
--- a/pkg/core/coretesting/expandable_resource.go
+++ b/pkg/core/coretesting/expandable_resource.go
@@ -10,7 +10,7 @@ import (
 
 type CreateCase[P any, R core.ExpandableResource[P]] struct {
 	Name     string
-	Existing R
+	Existing core.Resource
 	Params   P
 	Want     ResourcesExpectation
 	Check    func(assertions *assert.Assertions, resource R)
@@ -21,14 +21,12 @@ func (tt CreateCase[P, R]) Run(t *testing.T) {
 	assert := assert.New(t)
 
 	dag := core.NewResourceGraph()
-	// We have to use reflect, since Go doesn't know that R is a pointer, or even something whose zero-type is
-	// comparable (so we can't do "var R zero; if tt.Existing != zero").
-	if !reflect.ValueOf(tt.Existing).IsZero() {
+	if tt.Existing != nil {
 		dag.AddResource(tt.Existing)
 	}
 
 	var res R
-	rType := reflect.TypeOf(tt.Existing)
+	rType := reflect.TypeOf(res)
 	if rType.Kind() == reflect.Pointer {
 		res = reflect.New(rType.Elem()).Interface().(R)
 	}
diff --git a/pkg/core/coretesting/resources.go b/pkg/core/coretesting/resources.go
index 6db0bd58..a646a2ee 100644
--- a/pkg/core/coretesting/resources.go
+++ b/pkg/core/coretesting/resources.go
@@ -58,10 +58,8 @@ func (expect ResourcesExpectation) ElementsMatchPretty(t *testing.T, expected an
 
 	expectedStr := toStr(expected)
 	actualStr := toStr(actual)
-	if expectedStr != "" && actualStr != "" {
-		if !assert.Equal(t, expectedStr, actualStr) {
-			return
-		}
+	if !assert.Equal(t, expectedStr, actualStr) {
+		return
 	}
 
 	assert.ElementsMatch(t, expected, actual)
diff --git a/pkg/provider/aws/plugin_test.go b/pkg/provider/aws/plugin_test.go
index c72de158..9a1ce5b4 100644
--- a/pkg/provider/aws/plugin_test.go
+++ b/pkg/provider/aws/plugin_test.go
@@ -276,3 +276,79 @@ func Test_configureResources(t *testing.T) {
 		})
 	}
 }
+
+func Test_getS3BucketConfig(t *testing.T) {
+	cases := []struct {
+		name       string
+		constructs []core.Construct
+		want       resources.S3BucketConfigureParams
+		wantErr    bool
+	}{
+		{
+			name: "no constructs", // Not an expected case, but it should work anyway
+			want: getFsConfiguration(),
+		},
+		{
+			name: "fs construct",
+			constructs: []core.Construct{
+				&core.Fs{AnnotationKey: core.AnnotationKey{Capability: "fs", ID: "fs-a"}},
+			},
+			want: getFsConfiguration(),
+		},
+		{
+			name: "multiple exec units", // for lambda payloads
+			constructs: []core.Construct{
+				&core.Fs{AnnotationKey: core.AnnotationKey{Capability: "exec-proxy", ID: "unit-a"}},
+				&core.Fs{AnnotationKey: core.AnnotationKey{Capability: "exec-proxy", ID: "unit-b"}},
+			},
+			want: getFsConfiguration(),
+		},
+		{
+			name: "single static unit",
+			constructs: []core.Construct{
+				&core.StaticUnit{
+					AnnotationKey: core.AnnotationKey{Capability: "exec-proxy", ID: "unit-a"},
+					IndexDocument: "my index doc",
+				},
+			},
+			want: resources.S3BucketConfigureParams{
+				ForceDestroy:  true,
+				IndexDocument: "my index doc",
+			},
+		},
+		{
+			name: "multiple static units",
+			constructs: []core.Construct{
+				&core.StaticUnit{
+					AnnotationKey: core.AnnotationKey{Capability: "static", ID: "unit-a"},
+					IndexDocument: "my index doc 001",
+				},
+				&core.StaticUnit{
+					AnnotationKey: core.AnnotationKey{Capability: "static", ID: "unit-b"},
+					IndexDocument: "my index doc 002",
+				},
+			},
+			wantErr: true,
+		},
+	}
+	for _, tt := range cases {
+		t.Run(tt.name, func(t *testing.T) {
+			assert := assert.New(t)
+
+			bucket := &resources.S3Bucket{}
+			constructs := core.NewConstructGraph()
+			for _, cons := range tt.constructs {
+				constructs.AddConstruct(cons)
+				bucket.ConstructsRef.Add(cons.Provenance())
+			}
+
+			actualConfig, actualErr := getS3BucketConfig(bucket, constructs)
+			if tt.wantErr {
+				assert.Error(actualErr)
+			} else {
+				assert.NoError(actualErr)
+				assert.Equal(tt.want, actualConfig)
+			}
+		})
+	}
+}
diff --git a/pkg/provider/aws/resources/s3_test.go b/pkg/provider/aws/resources/s3_test.go
index 778bf35f..4cd8dd17 100644
--- a/pkg/provider/aws/resources/s3_test.go
+++ b/pkg/provider/aws/resources/s3_test.go
@@ -63,3 +63,56 @@ func Test_S3BucketCreate(t *testing.T) {
 		})
 	}
 }
+
+func Test_S3ObjectCreate(t *testing.T) {
+	annotationKey := core.AnnotationKey{ID: "test", Capability: annotation.StaticUnitCapability}
+	cases := []coretesting.CreateCase[S3ObjectCreateParams, *S3Object]{
+		{
+			Name: "s3 bucket missing",
+			Want: coretesting.ResourcesExpectation{
+				Nodes: []string{
+					"aws:s3_object:my-app-test-payloads",
+					"aws:s3_bucket:my-app-test",
+				},
+				Deps: []coretesting.StringDep{
+					{Source: "aws:s3_object:my-app-test-payloads", Destination: "aws:s3_bucket:my-app-test"},
+				},
+			},
+			Check: func(assertions *assert.Assertions, object *S3Object) {
+				// nothing extra
+			},
+			WantErr: false,
+		},
+		{
+			Name:     "s3 bucket alrady there",
+			Existing: &S3Bucket{Name: "my-app-test"},
+			Want: coretesting.ResourcesExpectation{
+				Nodes: []string{
+					"aws:s3_object:my-app-test-payloads",
+					"aws:s3_bucket:my-app-test",
+				},
+				Deps: []coretesting.StringDep{
+					{Source: "aws:s3_object:my-app-test-payloads", Destination: "aws:s3_bucket:my-app-test"},
+				},
+			},
+			Check: func(assertions *assert.Assertions, object *S3Object) {
+				// nothing extra
+			},
+			WantErr: false,
+		},
+	}
+	for _, tt := range cases {
+		t.Run(tt.Name, func(t *testing.T) {
+			tt.Params = S3ObjectCreateParams{
+				AppName:    "my-app",
+				Refs:       core.AnnotationKeySetOf(annotationKey),
+				BucketName: annotationKey.ID,
+				Name:       "payloads",
+				Key:        "object-key",
+				FilePath:   "local/path.txt",
+			}
+			tt.Run(t)
+		})
+	}
+
+}
diff --git a/pkg/provider/aws/static_unit_test.go b/pkg/provider/aws/static_unit_test.go
deleted file mode 100644
index 4d02b0e3..00000000
--- a/pkg/provider/aws/static_unit_test.go
+++ /dev/null
@@ -1,83 +0,0 @@
-package aws
-
-import (
-	"testing"
-
-	"github.com/klothoplatform/klotho/pkg/annotation"
-	"github.com/klothoplatform/klotho/pkg/config"
-	"github.com/klothoplatform/klotho/pkg/core"
-	"github.com/klothoplatform/klotho/pkg/core/coretesting"
-	"github.com/klothoplatform/klotho/pkg/graph"
-	"github.com/stretchr/testify/assert"
-)
-
-func Test_GenerateStaticUnitResources(t *testing.T) {
-	unit := &core.StaticUnit{AnnotationKey: core.AnnotationKey{ID: "test", Capability: annotation.ExecutionUnitCapability}}
-	unit.AddSharedFile(&core.FileRef{FPath: "test/Shared"})
-	unit.AddStaticFile(&core.FileRef{FPath: "test/Static"})
-	cases := []struct {
-		name          string
-		indexDocument string
-		cfg           config.Application
-		want          coretesting.ResourcesExpectation
-		wantErr       bool
-	}{
-		{
-			name: "generate static unit with no index document",
-			cfg: config.Application{
-				AppName: "test-app",
-			},
-			want: coretesting.ResourcesExpectation{
-				Nodes: []string{
-					"aws:s3_bucket:test-app-test",
-					"aws:s3_object:test-app-test-Shared",
-					"aws:s3_object:test-app-test-Static",
-				},
-				Deps: []graph.Edge[string]{
-					{Source: "aws:s3_object:test-app-test-Shared", Destination: "aws:s3_bucket:test-app-test"},
-					{Source: "aws:s3_object:test-app-test-Static", Destination: "aws:s3_bucket:test-app-test"},
-				},
-			},
-		},
-		{
-			name: "generate static unit with index document as website",
-			cfg: config.Application{
-				AppName: "test-app",
-			},
-			indexDocument: "index.html",
-			want: coretesting.ResourcesExpectation{
-				Nodes: []string{
-					"aws:s3_bucket:test-app-test",
-					"aws:s3_object:test-app-test-Shared",
-					"aws:s3_object:test-app-test-Static",
-				},
-				Deps: []graph.Edge[string]{
-					{Source: "aws:s3_object:test-app-test-Shared", Destination: "aws:s3_bucket:test-app-test"},
-					{Source: "aws:s3_object:test-app-test-Static", Destination: "aws:s3_bucket:test-app-test"},
-				},
-			},
-		},
-	}
-	for _, tt := range cases {
-		t.Run(tt.name, func(t *testing.T) {
-			assert := assert.New(t)
-			aws := AWS{
-				Config: &tt.cfg,
-			}
-			dag := core.NewResourceGraph()
-			unit.IndexDocument = tt.indexDocument
-
-			err := aws.GenerateStaticUnitResources(unit, dag)
-			if tt.wantErr {
-				assert.Error(err)
-				return
-			}
-			if !assert.NoError(err) {
-				return
-			}
-
-			tt.want.Assert(t, dag)
-		})
-
-	}
-}