Merge pull request #48 from posit-dev/ppm-storage-region

Set S3 Storage Region in Package Manager and Chronicle

Author: timtalbot

.claude/settings.json

@@ -3,7 +3,7 @@
     "allow": [
       "Bash(just test)",
       "Bash(just build)",
-      "Bash(make *)"
+      "Bash(make :*)"
     ]
   },
   "hooks": {

.gitignore

@@ -17,6 +17,9 @@ coverage.*
 *.coverprofile
 profile.cov
 
+# Build artifacts
+bin/
+
 # Dependency directories (remove the comment below to include it)
 # vendor/
 

api/product/secret.go

@@ -4,10 +4,8 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
-	"os"
 
 	"github.com/aws/aws-sdk-go/aws"
-	"github.com/aws/aws-sdk-go/aws/endpoints"
 	"github.com/aws/aws-sdk-go/aws/session"
 	"github.com/aws/aws-sdk-go/service/secretsmanager"
 	"github.com/pkg/errors"
@@ -121,7 +119,7 @@ func GetSecretProviderClassForAllSecrets(p KubernetesOwnerProvider, name, namesp
 				SecretObjects: generateSecretObjects(p, kubernetesSecrets),
 				Parameters: map[string]string{
 					"objects": secretObjectYaml,
-					"region":  getAWSRegion(),
+					"region":  GetAWSRegion(),
 				},
 			},
 		}, nil
@@ -137,26 +135,12 @@ const (
 	SiteSecretNone       SiteSecretType = ""
 )
 
-// getAWSRegion returns the AWS region to use for secret operations.
-// It checks the AWS_REGION environment variable first, then falls back to AWS_DEFAULT_REGION,
-// and finally defaults to us-east-2 for backwards compatibility.
-func getAWSRegion() string {
-	if region := os.Getenv("AWS_REGION"); region != "" {
-		return region
-	}
-	if region := os.Getenv("AWS_DEFAULT_REGION"); region != "" {
-		return region
-	}
-	// Fallback to the original hardcoded region for backwards compatibility
-	return endpoints.UsEast2RegionID
-}
-
 func FetchSecret(ctx context.Context, r SomeReconciler, req ctrl.Request, secretType SiteSecretType, vaultName, key string) (string, error) {
 	l := r.GetLogger(ctx)
 	switch secretType {
 	case SiteSecretAws:
 		if sess, err := session.NewSession(&aws.Config{
-			Region: aws.String(getAWSRegion()),
+			Region: aws.String(GetAWSRegion()),
 		}); err != nil {
 			return "", err
 		} else {

api/product/util.go

@@ -3,8 +3,10 @@ package product
 import (
 	"crypto/sha256"
 	"fmt"
+	"os"
 	"sort"
 
+	"github.com/aws/aws-sdk-go/aws/endpoints"
 	"golang.org/x/exp/maps"
 	corev1 "k8s.io/api/core/v1"
 )
@@ -89,3 +91,17 @@ func ComputeSha256(in map[string]string) (string, error) {
 		return fmt.Sprintf("%x", h.Sum(nil)), nil
 	}
 }
+
+// GetAWSRegion returns the AWS region.
+// It checks the AWS_REGION environment variable first, then falls back to AWS_DEFAULT_REGION,
+// and finally defaults to us-east-2 for backwards compatibility.
+func GetAWSRegion() string {
+	if region := os.Getenv("AWS_REGION"); region != "" {
+		return region
+	}
+	if region := os.Getenv("AWS_DEFAULT_REGION"); region != "" {
+		return region
+	}
+	// Fallback to the original hardcoded region for backwards compatibility
+	return endpoints.UsEast2RegionID
+}

api/product/util_test.go

@@ -1,8 +1,10 @@
 package product_test
 
 import (
+	"os"
 	"testing"
 
+	"github.com/aws/aws-sdk-go/aws/endpoints"
 	"github.com/posit-dev/team-operator/api/product"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -80,3 +82,54 @@ func TestLabelMerge(t *testing.T) {
 	expected = map[string]string{"vorpal": "sword"}
 	assert.Equal(t, expected, result)
 }
+
+func TestGetAWSRegion(t *testing.T) {
+	tests := []struct {
+		name       string
+		awsRegion  string
+		awsDefault string
+		want       string
+	}{
+		{"AWS_REGION set", "us-west-2", "", "us-west-2"},
+		{"AWS_DEFAULT_REGION set", "", "eu-west-1", "eu-west-1"},
+		{"Both set, AWS_REGION wins", "us-west-2", "eu-west-1", "us-west-2"},
+		{"Neither set, defaults", "", "", endpoints.UsEast2RegionID},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			// Save original values
+			origRegion := os.Getenv("AWS_REGION")
+			origDefault := os.Getenv("AWS_DEFAULT_REGION")
+			defer func() {
+				// Restore original values
+				if origRegion != "" {
+					os.Setenv("AWS_REGION", origRegion)
+				} else {
+					os.Unsetenv("AWS_REGION")
+				}
+				if origDefault != "" {
+					os.Setenv("AWS_DEFAULT_REGION", origDefault)
+				} else {
+					os.Unsetenv("AWS_DEFAULT_REGION")
+				}
+			}()
+
+			// Set test values
+			if tt.awsRegion != "" {
+				os.Setenv("AWS_REGION", tt.awsRegion)
+			} else {
+				os.Unsetenv("AWS_REGION")
+			}
+			if tt.awsDefault != "" {
+				os.Setenv("AWS_DEFAULT_REGION", tt.awsDefault)
+			} else {
+				os.Unsetenv("AWS_DEFAULT_REGION")
+			}
+
+			if got := product.GetAWSRegion(); got != tt.want {
+				t.Errorf("GetAWSRegion() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}

internal/controller/core/site_controller_chronicle.go

@@ -6,8 +6,8 @@ import (
 	"github.com/posit-dev/team-operator/api/core/v1beta1"
 	"github.com/posit-dev/team-operator/api/product"
 	"github.com/posit-dev/team-operator/internal"
-	"k8s.io/apimachinery/pkg/apis/meta/v1"
-	"sigs.k8s.io/controller-runtime"
+	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	controllerruntime "sigs.k8s.io/controller-runtime"
 )
 
 func (r *SiteReconciler) reconcileChronicle(ctx context.Context, req controllerruntime.Request, site *v1beta1.Site) error {
@@ -83,8 +83,7 @@ func (r *SiteReconciler) reconcileChronicle(ctx context.Context, req controllerr
 				Enabled: true,
 				Bucket:  site.Spec.Chronicle.S3Bucket,
 				Prefix:  site.Name + "/chr-v0",
-				// TODO: should not be hard-coded
-				Region: "us-east-2",
+				Region:  product.GetAWSRegion(),
 			}
 		}
 		return nil

internal/controller/core/site_controller_package_manager.go

@@ -111,6 +111,7 @@ func (r *SiteReconciler) reconcilePackageManager(
 			}
 			pm.Spec.Config.S3Storage.Bucket = site.Spec.PackageManager.S3Bucket
 			pm.Spec.Config.S3Storage.Prefix = site.Name + "/ppm-v0"
+			pm.Spec.Config.S3Storage.Region = product.GetAWSRegion()
 		}
 		return nil
 	}); err != nil {