Merge pull request #116 from gympass/PE1-3200/prefix-bucket

[PE1-3200] feat: optionally send logs to specific directory in S3 Bucket

Author: LCaparelli

.env.example

@@ -9,6 +9,7 @@ CF_CUSTOM_SSL_CERT="arn:aws:acm:us-east-1:123456789012:certificate/473e64fd-78bc
 CF_SECURITY_POLICY="TLSv1.2_2021"
 CF_ENABLE_LOGGING="true"
 CF_S3_BUCKET_LOG="mys3bucket.s3.amazonaws.com"
+CF_S3_BUCKET_LOG_PREFIX="cloudfront"
 CF_ENABLE_IPV6="true"
 CF_DESCRIPTION_TEMPLATE="Serve contents for {{group}} group."
 CF_ROUTE53_CREATE_ALIAS="true"

README.md

@@ -338,6 +338,7 @@ Use the following environment variables to change the controller's behavior:
 | CF_ENABLE_LOGGING         | No       | If set to true enables sending logs to CloudWatch; `CF_S3_BUCKET_LOG` must be set as well.                                                                                                                                                                                                                                                                   | "false"                               |
 | CF_PRICE_CLASS            | Yes      | The distribution price class. Possible values are: "PriceClass_All", "PriceClass_200", "PriceClass_100". [Official reference](https://docs.aws.amazon.com/AmazonCloudFront/latest/DeveloperGuide/PriceClass.html).                                                                                                                                           | "PriceClass_All"                      |
 | CF_S3_BUCKET_LOG          | No       | The domain of the S3 bucket CloudWatch logs should be sent to. Each distribution will have its own directory inside the bucket with the same as the distribution's group. For example, if the group is "foo", the logs will be stored as `foo/<ID>.<timestamp and hash>.gz`.<br><br> If `CF_ENABLE_LOGGING` is not set to "true" then this value is ignored. | ""                                    |
+| CF_S3_BUCKET_LOG_PREFIX   | No       | The directory within the S3 bucket informed in `CF_S3_BUCKET_LOG` logs should be created in. For example, if set to `"foo/bar"`, logs from a group called "group" will be stored in `foo/bar/group` in the S3 bucket. Trailing slash is ignore on the value, if informed (eg, "foo/bar/" ends up as "foo/bar").                                              | ""                                    |
 | CF_SECURITY_POLICY        | No       | The TLS/SSL security policy to be used when serving requests. [Official reference](https://docs.aws.amazon.com/AmazonCloudFront/latest/DeveloperGuide/secure-connections-supported-viewer-protocols-ciphers.html). <br><br> Must also inform a valid `CF_CUSTOM_SSL_CERT` if set.                                                                            | ""                                    |
 | DEV_MODE                  | No       | When set to "true" logs in unstructured text instead of JSON. Also overrides LOG_LEVEL to "debug".                                                                                                                                                                                                                                                           | "false"                               |
 | LOG_LEVEL                 | No       | Represents log level of verbosity. Can be "debug", "info", "warn", "error", "dpanic", "panic" and "fatal" (sorted with decreasing verbosity).                                                                                                                                                                                                                | "info"                                |

internal/cloudfront/repository.go

@@ -282,7 +282,7 @@ func (r DistRepository) waitUntilDeployed(id string) (*string, error) {
 	defer cancel()
 
 	var eTag *string
-	condition := func(ctx context.Context) (done bool, err error) {
+	condition := func(context.Context) (done bool, err error) {
 		out, err := r.distributionByID(id)
 		if err != nil {
 			if cdnaws.IsErrorCode(err, awscloudfront.ErrCodeNoSuchDistribution) {

internal/cloudfront/service.go

@@ -239,7 +239,7 @@ func (s *Service) newDistribution(ingresses []k8s.CDNIngress, group string, shar
 	}
 
 	if s.Config.CloudFrontEnableLogging && len(s.Config.CloudFrontS3BucketLog) > 0 {
-		b = b.WithLogging(s.Config.CloudFrontS3BucketLog, group)
+		b = b.WithLogging(s.Config.CloudFrontS3BucketLog, s.s3Prefix(group))
 	}
 
 	if len(s.Config.CloudFrontCustomTags) > 0 {
@@ -272,6 +272,13 @@ func (s *Service) discoverCert(ingresses []k8s.CDNIngress) (certificate.Certific
 	return certificate.Certificate{}, errs.ErrorOrNil()
 }
 
+func (s *Service) s3Prefix(group string) string {
+	if len(s.Config.CloudFrontS3BucketLogPrefix) == 0 {
+		return group
+	}
+	return fmt.Sprintf("%s/%s", s.Config.CloudFrontS3BucketLogPrefix, group)
+}
+
 func (s *Service) syncDist(ctx context.Context, desiredDist Distribution, cdnStatus *v1alpha1.CDNStatus, ing client.Object) (Distribution, error) {
 	if desiredDist.IsEmpty() {
 		return desiredDist, s.deleteDistribution(ctx, desiredDist)

internal/cloudfront/service_test.go

@@ -114,3 +114,19 @@ func (s *CloudFrontServiceTestSuite) Test_validateCreation_ExistingDistributions
 
 	s.NoError(svc.validateCreation(dist, ing))
 }
+
+func (s *CloudFrontServiceTestSuite) Test_s3Prefix_NoPrefixShouldJustUseGroup() {
+	svc := Service{Config: config.Config{
+		CloudFrontS3BucketLogPrefix: "",
+	}}
+
+	s.Equal("group", svc.s3Prefix("group"))
+}
+
+func (s *CloudFrontServiceTestSuite) Test_s3Prefix_PrefixSetShouldConcatenateWithGroup() {
+	svc := Service{Config: config.Config{
+		CloudFrontS3BucketLogPrefix: "foo/bar",
+	}}
+
+	s.Equal("foo/bar/group", svc.s3Prefix("group"))
+}

internal/config/config.go

@@ -39,6 +39,7 @@ const (
 	cfSecurityPolicyKey                           = "cf_security_policy"
 	cfEnableLoggingKey                            = "cf_enable_logging"
 	cfS3BucketLogKey                              = "cf_s3_bucket_log"
+	cfS3BucketLogPrefixKey                        = "cf_s3_bucket_log_prefix"
 	cfEnableIPV6Key                               = "cf_enable_ipv6"
 	cfDescriptionTemplateKey                      = "cf_description_template"
 	cfCustomTagsKey                               = "cf_custom_tags"
@@ -64,6 +65,7 @@ func initDefaults() {
 	viper.SetDefault(cfSecurityPolicyKey, "")
 	viper.SetDefault(cfEnableLoggingKey, "false")
 	viper.SetDefault(cfS3BucketLogKey, "")
+	viper.SetDefault(cfS3BucketLogPrefixKey, "")
 	viper.SetDefault(cfEnableIPV6Key, "true")
 	viper.SetDefault(cfDescriptionTemplateKey, "Serve contents for {{group}} group.")
 	viper.SetDefault(cfCustomTagsKey, "")
@@ -106,6 +108,9 @@ type Config struct {
 	CloudFrontEnableLogging bool
 	// CloudFrontS3BucketLog if logging enabled represents the S3 Bucket URL to persists, for example myawslogbucket.s3.amazonaws.com.
 	CloudFrontS3BucketLog string
+	// CloudFrontS3BucketLogPrefix is the prefix that should be added to the S3 path when sending logs. The directory on which logs should be stored.
+	// Trailing slashes are ignored ("foo/bar/" is the same as "foo/bar").
+	CloudFrontS3BucketLogPrefix string
 	// CloudFrontEnableIPV6 if should enable ipv6 for distribution responses.
 	CloudFrontEnableIPV6 bool
 	// CloudFrontDescriptionTemplate the description template for distribution.
@@ -174,6 +179,7 @@ func Parse() (Config, error) {
 		CloudFrontSecurityPolicy:              viper.GetString(cfSecurityPolicyKey),
 		CloudFrontEnableLogging:               viper.GetBool(cfEnableLoggingKey),
 		CloudFrontS3BucketLog:                 viper.GetString(cfS3BucketLogKey),
+		CloudFrontS3BucketLogPrefix:           removeTrailingSlash(viper.GetString(cfS3BucketLogPrefixKey)),
 		CloudFrontEnableIPV6:                  viper.GetBool(cfEnableIPV6Key),
 		CloudFrontDescriptionTemplate:         viper.GetString(cfDescriptionTemplateKey),
 		CloudFrontCustomTags:                  extractTags(viper.GetString(cfCustomTagsKey)),
@@ -186,6 +192,13 @@ func Parse() (Config, error) {
 	}, nil
 }
 
+func removeTrailingSlash(getString string) string {
+	if strings.HasSuffix(getString, "/") {
+		return getString[:len(getString)-1]
+	}
+	return getString
+}
+
 func parseNamespacedNames(names []string) ([]types.NamespacedName, error) {
 	var result []types.NamespacedName
 	for _, n := range names {

internal/config/config_test.go

@@ -74,6 +74,31 @@ func (s *ConfigTestSuite) TestParse_DefaultToBlockCreationIsFalse() {
 	s.False(cfg.IsCreateBlocked)
 }
 
+func (s *ConfigTestSuite) TestParse_BucketPrefixIsSet() {
+	testCases := []struct {
+		name   string
+		prefix string
+	}{
+		{
+			name:   "No trailing slash",
+			prefix: "foo/bar",
+		},
+		{
+			name:   "With trailing slash",
+			prefix: "foo/bar/",
+		},
+	}
+
+	for _, tc := range testCases {
+		viper.Set("cf_s3_bucket_log_prefix", tc.prefix)
+
+		cfg, err := Parse()
+
+		s.NoErrorf(err, "test case: %s", tc.name)
+		s.Equalf("foo/bar", cfg.CloudFrontS3BucketLogPrefix, "test case: %s", tc.name)
+	}
+}
+
 func (s *ConfigTestSuite) TestIsCreationAllowed_UnblockedCreationReturnsTrue() {
 	viper.Set(createBlockedKey, "false")
 

internal/k8s/ingress_fetcher_v1_test.go

@@ -257,7 +257,7 @@ func (s *IngressFetcherV1TestSuite) TestFetchBy_FailureWithUserOrigins() {
 			Build()
 
 		fetcher := NewIngressFetcherV1(client)
-		predicate := func(ing CDNIngress) bool { return true }
+		predicate := func(CDNIngress) bool { return true }
 
 		got, err := fetcher.FetchBy(context.Background(), s.CDNClass, predicate)
 		s.Error(err, "test: %s", tc.name)