Access Log Policy API Reference, Annotation Prefixes, Permissions Doc…

…s, and Logging Improvements (#452) * Added Access Log Policy API reference and fixed AccessLogSubscription annotation key * Added missing permissions to recommended policy, added top-level reconciler error logs --------- Co-authored-by: Shawn Kaplan <[email protected]>
aws · Oct 26, 2023 · a2d5b97 · a2d5b97
1 parent a7fdbd9
commit a2d5b97
Show file tree

Hide file tree

Showing 13 changed files with 195 additions and 18 deletions.
diff --git a/config/iam/recommended-inline-policy.json b/config/iam/recommended-inline-policy.json
@@ -8,7 +8,13 @@
                 "iam:CreateServiceLinkedRole",
                 "ec2:DescribeVpcs",
                 "ec2:DescribeSubnets",
-                "ec2:DescribeTags"
+                "ec2:DescribeTags",
+                "ec2:DescribeSecurityGroups",
+                "logs:CreateLogDelivery",
+                "logs:GetLogDelivery",
+                "logs:UpdateLogDelivery",
+                "logs:DeleteLogDelivery",
+                "logs:ListLogDeliveries"
             ],
             "Resource": "*"
         }

diff --git a/controllers/accesslogpolicy_controller.go b/controllers/accesslogpolicy_controller.go
@@ -106,6 +106,9 @@ func RegisterAccessLogPolicyController(
 func (r *accessLogPolicyReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
 	r.log.Infow("reconcile", "name", req.Name)
 	recErr := r.reconcile(ctx, req)
+	if recErr != nil {
+		r.log.Infow("reconcile error", "name", req.Name, "message", recErr.Error())
+	}
 	res, retryErr := lattice_runtime.HandleReconcileError(recErr)
 	if res.RequeueAfter != 0 {
 		r.log.Infow("requeue request", "name", req.Name, "requeueAfter", res.RequeueAfter)

diff --git a/controllers/gateway_controller.go b/controllers/gateway_controller.go
@@ -19,6 +19,7 @@ package controllers
 import (
 	"context"
 	"fmt"
+
 	anv1alpha1 "github.com/aws/aws-application-networking-k8s/pkg/apis/applicationnetworking/v1alpha1"
 
 	"github.com/aws/aws-application-networking-k8s/pkg/aws"
@@ -114,6 +115,9 @@ func RegisterGatewayController(
 func (r *gatewayReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
 	r.log.Infow("reconcile", "name", req.Name)
 	recErr := r.reconcile(ctx, req)
+	if recErr != nil {
+		r.log.Infow("reconcile error", "name", req.Name, "message", recErr.Error())
+	}
 	res, retryErr := lattice_runtime.HandleReconcileError(recErr)
 	if res.RequeueAfter != 0 {
 		r.log.Infow("requeue request", "name", req.Name, "requeueAfter", res.RequeueAfter)

diff --git a/controllers/route_controller.go b/controllers/route_controller.go
@@ -161,12 +161,15 @@ func RegisterAllRouteControllers(
 //+kubebuilder:rbac:groups=gateway.networking.k8s.io,resources=grpcroutes/finalizers;httproutes/finalizers,verbs=update
 
 func (r *routeReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
-	return lattice_runtime.HandleReconcileError(r.reconcile(ctx, req))
+	r.log.Infow("reconcile", "name", req.Name)
+	recErr := r.reconcile(ctx, req)
+	if recErr != nil {
+		r.log.Infow("reconcile error", "name", req.Name, "message", recErr.Error())
+	}
+	return lattice_runtime.HandleReconcileError(recErr)
 }
 
 func (r *routeReconciler) reconcile(ctx context.Context, req ctrl.Request) error {
-	r.log.Infow("reconcile", "name", req.Name)
-
 	route, err := r.getRoute(ctx, req)
 	if err != nil {
 		return client.IgnoreNotFound(err)

diff --git a/controllers/service_controller.go b/controllers/service_controller.go
@@ -91,12 +91,15 @@ func RegisterServiceController(
 //+kubebuilder:rbac:groups=core,resources=configmaps, verbs=create;delete;patch;update;get;list;watch
 
 func (r *serviceReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
-	return lattice_runtime.HandleReconcileError(r.reconcile(ctx, req))
+	r.log.Infow("reconcile", "name", req.Name)
+	recErr := r.reconcile(ctx, req)
+	if recErr != nil {
+		r.log.Infow("reconcile error", "name", req.Name, "message", recErr.Error())
+	}
+	return lattice_runtime.HandleReconcileError(recErr)
 }
 
 func (r *serviceReconciler) reconcile(ctx context.Context, req ctrl.Request) error {
-	r.log.Infow("reconcile", "name", req.Name)
-
 	svc := &corev1.Service{}
 	if err := r.client.Get(ctx, req.NamespacedName, svc); err != nil {
 		return client.IgnoreNotFound(err)

diff --git a/controllers/serviceexport_controller.go b/controllers/serviceexport_controller.go
@@ -107,7 +107,12 @@ func RegisterServiceExportController(
 //+kubebuilder:rbac:groups=multicluster.x-k8s.io,resources=serviceexports/finalizers,verbs=update
 
 func (r *serviceExportReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
-	return lattice_runtime.HandleReconcileError(r.reconcile(ctx, req))
+	r.log.Infow("reconcile", "name", req.Name)
+	recErr := r.reconcile(ctx, req)
+	if recErr != nil {
+		r.log.Infow("reconcile error", "name", req.Name, "message", recErr.Error())
+	}
+	return lattice_runtime.HandleReconcileError(recErr)
 }
 
 func (r *serviceExportReconciler) reconcile(ctx context.Context, req ctrl.Request) error {

diff --git a/controllers/serviceimport_controller.go b/controllers/serviceimport_controller.go
@@ -27,7 +27,6 @@ import (
 	"k8s.io/client-go/tools/record"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
-	"sigs.k8s.io/controller-runtime/pkg/log"
 	mcs_api "sigs.k8s.io/mcs-api/pkg/apis/v1alpha1"
 
 	"github.com/aws/aws-application-networking-k8s/pkg/k8s"
@@ -76,27 +75,25 @@ func RegisterServiceImportController(
 //+kubebuilder:rbac:groups=multicluster.x-k8s.io,resources=serviceimports/finalizers,verbs=update
 
 func (r *serviceImportReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
-	reconcileLog := log.FromContext(ctx)
-
-	reconcileLog.Info("serviceImportReconciler")
+	r.log.Infow("reconcile", "name", req.Name)
 
 	serviceImport := &mcs_api.ServiceImport{}
 
 	if err := r.client.Get(ctx, req.NamespacedName, serviceImport); err != nil {
-		reconcileLog.Info("Item Not Found")
+		r.log.Info("Item Not Found")
 		return ctrl.Result{}, nil
 	}
 
 	if !serviceImport.DeletionTimestamp.IsZero() {
-		reconcileLog.Info("Deleting")
+		r.log.Info("Deleting")
 		r.finalizerManager.RemoveFinalizers(ctx, serviceImport, serviceImportFinalizer)
 		return ctrl.Result{}, nil
 	} else {
 		if err := r.finalizerManager.AddFinalizers(ctx, serviceImport, serviceImportFinalizer); err != nil {
 			r.eventRecorder.Event(serviceImport, corev1.EventTypeWarning, k8s.ServiceImportEventReasonFailedAddFinalizer, fmt.Sprintf("Failed add finalizer due to %v", err))
 			return ctrl.Result{}, nil
 		}
-		reconcileLog.Info("Adding/Updating")
+		r.log.Info("Adding/Updating")
 
 		return ctrl.Result{}, nil
 	}

diff --git a/docs/deploy.md b/docs/deploy.md
@@ -37,7 +37,13 @@ Run through them again for a second cluster to use with the extended example sho
                    "iam:CreateServiceLinkedRole",
                    "ec2:DescribeVpcs",
                    "ec2:DescribeSubnets",
-                   "ec2:DescribeTags"
+                   "ec2:DescribeTags",
+                   "ec2:DescribeSecurityGroups",
+                   "logs:CreateLogDelivery",
+                   "logs:GetLogDelivery",
+                   "logs:UpdateLogDelivery",
+                   "logs:DeleteLogDelivery",
+                   "logs:ListLogDeliveries"
                ],
                "Resource": "*"
            }

diff --git a/docs/reference/access-log-policy.md b/docs/reference/access-log-policy.md
@@ -0,0 +1,141 @@
+# AccessLogPolicy API Reference
+
+## Introduction
+
+The AccessLogPolicy custom resource allows you to define access logging configurations on
+Gateways, HTTPRoutes, and GRPCRoutes by specifying a destination for the access logs to be published to.
+
+## Features
+- When an AccessLogPolicy is created for a Gateway target, VPC Lattice traffic to any Route that is a child of that Gateway will have access logs published to the provided destination
+- When an AccessLogPolicy is created for an HTTPRoute or GRPCRoute target, VPC Lattice traffic to that Route will have access logs published to the provided destination
+
+## Definition
+
+| Field        | Type                                                                                                     | Description                                      |
+|--------------|----------------------------------------------------------------------------------------------------------|--------------------------------------------------|
+| `apiVersion` | *string*                                                                                                 | `application-networking.k8s.aws/v1alpha1`       |
+| `kind`       | *string*                                                                                                 | `AccessLogPolicy`                                |
+| `metadata`   | [*ObjectMeta*](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.26/#objectmeta-v1-meta) | Kubernetes metadata for the resource.            |
+| `spec`       | *AccessLogPolicySpec*                                                                                    | Defines the desired state of AccessLogPolicy.    |
+
+### AccessLogPolicySpec
+
+| Field                                       | Type                                                                                           | Description                                                                                                                                           |
+|---------------------------------------------|------------------------------------------------------------------------------------------------|-------------------------------------------------------------------------------------------------------------------------------------------------------|
+| `destinationArn`                            | *string*                                                                                       | The ARN of the Amazon S3 Bucket, Amazon CloudWatch Log Group, or Amazon Kinesis Data Firehose Delivery Stream that will have access logs published to it. |
+| `targetRef`                                 | *[PolicyTargetReference](https://gateway-api.sigs.k8s.io/geps/gep-713/#policy-targetref-api)* | TargetRef points to the kubernetes `Gateway`, `HTTPRoute`, or `GRPCRoute` resource that will have this policy attached. This field is following the guidelines of Kubernetes Gateway API policy attachment. |
+
+## Example Configurations
+
+### Example 1
+
+This configuration results in access logs being published to the S3 Bucket, `my-bucket`, when traffic
+is sent to any HTTPRoute or GRPCRoute that is a child of Gateway `my-hotel`.
+
+```yaml
+apiVersion: application-networking.k8s.aws/v1alpha1
+kind: AccessLogPolicy
+metadata:
+  name: my-access-log-policy
+spec:
+  destinationArn: "arn:aws:s3:::my-bucket"
+  targetRef:
+    group: gateway.networking.k8s.io
+    kind: Gateway
+    name: my-hotel
+```
+
+### Example 2
+
+This configuration results in access logs being published to the CloudWatch Log Group, `myloggroup`, when traffic
+is sent to HTTPRoute `inventory`.
+
+```yaml
+apiVersion: application-networking.k8s.aws/v1alpha1
+kind: AccessLogPolicy
+metadata:
+  name: my-access-log-policy
+spec:
+  destinationArn: "arn:aws:logs:us-west-2:123456789012:log-group:myloggroup:*"
+  targetRef:
+    group: gateway.networking.k8s.io
+    kind: HTTPRoute
+    name: inventory
+```
+
+## AWS Permissions Required
+
+Per the [VPC Lattice documentation](https://docs.aws.amazon.com/vpc-lattice/latest/ug/monitoring-access-logs.html#monitoring-access-logs-IAM),
+ IAM permissions are required to enable access logs:
+
+```json
+{
+    "Version": "2012-10-17",
+    "Statement": [
+        {
+            "Effect": "Allow",
+            "Sid": "ManageVPCLatticeAccessLogSetup",
+            "Action": [
+                "logs:CreateLogDelivery",
+                "logs:GetLogDelivery",
+                "logs:UpdateLogDelivery",
+                "logs:DeleteLogDelivery",
+                "logs:ListLogDeliveries",
+                "vpc-lattice:CreateAccessLogSubscription",
+                "vpc-lattice:GetAccessLogSubscription",
+                "vpc-lattice:UpdateAccessLogSubscription",
+                "vpc-lattice:DeleteAccessLogSubscription",
+                "vpc-lattice:ListAccessLogSubscriptions"
+            ],
+            "Resource": [
+                "*"
+            ]
+        }
+    ]
+}
+```
+
+## Statuses
+
+AccessLogPolicies fit under the definition of [Gateway API Policy Objects](https://gateway-api.sigs.k8s.io/geps/gep-713/#on-policy-objects).
+As a result, status conditions are applied on every modification of an AccessLogPolicy, and can be viewed by describing it.
+
+### Status Condition Reasons
+
+#### Accepted
+
+The spec of the AccessLogPolicy is valid and has been successfully processed.
+
+#### Conflicted
+
+The target already has an AccessLogPolicy for the same destination type
+(i.e. a target can have 1 AccessLogPolicy for an S3 Bucket, 1 for a CloudWatch Log Group,
+and 1 for a Firehose Delivery Stream at a time).
+
+#### Invalid
+
+Any of the following:
+- The target's `Group` is not `gateway.networking.k8s.io`
+- The target's `Kind` is not `Gateway`, `HTTPRoute`, or `GRPCRoute`
+- The target's namespace does not match the AccessLogPolicy's namespace
+- The destination with the given `destinationArn` could not be found
+- The controller is missing AWS permissions
+
+#### TargetNotFound
+
+The target does not exist.
+
+## Annotations
+
+Upon successful creation or modification of an AccessLogPolicy, the controller may add or update an annotation in the
+AccessLogPolicy. The annotation applied by the controller has the key
+`application-networking.k8s.aws/accessLogSubscription`, and its value is the corresponding VPC Lattice Access Log
+Subscription's ARN.
+
+When an AccessLogPolicy's `destinationArn` is changed such that the resource type changes (e.g. from S3 Bucket to CloudWatch Log Group),
+or the AccessLogPolicy's `targetRef` is changed, the annotation's value will be updated because a new Access Log Subscription will be created to replace the previous one.
+
+When creation of an AccessLogPolicy fails, no annotation is added to the AccessLogPolicy because no corresponding Access Log Subscription exists.
+
+When modification or deletion of an AccessLogPolicy fails, the previous value of the annotation is left unchanged because the
+corresponding Access Log Subscription is also left unchanged.
diff --git a/examples/recommended-inline-policy.json b/examples/recommended-inline-policy.json
@@ -7,7 +7,13 @@
                 "vpc-lattice:*",
                 "ec2:DescribeVpcs",
                 "ec2:DescribeSubnets",
-                "ec2:DescribeTags"
+                "ec2:DescribeTags",
+                "ec2:DescribeSecurityGroups",
+                "logs:CreateLogDelivery",
+                "logs:GetLogDelivery",
+                "logs:UpdateLogDelivery",
+                "logs:DeleteLogDelivery",
+                "logs:ListLogDeliveries"
             ],
             "Resource": "*"
         },

diff --git a/mkdocs.yml b/mkdocs.yml
@@ -20,6 +20,7 @@ nav:
     - GRPCRoute: reference/grpc-route.md
     - TargetGroupPolicy: reference/target-group-policy.md
     - VpcAssociationPolicy: reference/vpc-association-policy.md
+    - AccessLogPolicy: reference/access-log-policy.md
   - Design Overview: overview.md
 
 plugins:

diff --git a/pkg/apis/applicationnetworking/v1alpha1/accesslogpolicy_types.go b/pkg/apis/applicationnetworking/v1alpha1/accesslogpolicy_types.go
@@ -10,7 +10,7 @@ import (
 )
 
 const (
-	AccessLogSubscriptionAnnotationKey = "VpcLatticeAccessLogSubscription"
+	AccessLogSubscriptionAnnotationKey = k8s.AnnotationPrefix + "accessLogSubscription"
 )
 
 // +genclient

diff --git a/pkg/k8s/utils.go b/pkg/k8s/utils.go
@@ -8,6 +8,8 @@ import (
 	ctrl "sigs.k8s.io/controller-runtime"
 )
 
+const AnnotationPrefix = "application-networking.k8s.aws/"
+
 type NamespacedAndNamed interface {
 	GetNamespace() string
 	GetName() string
-Original file line number
+Diff line change
@@ Expand Up / @@ -10,7 +10,7 @@ import ( @@
     )
     const (
-    	AccessLogSubscriptionAnnotationKey = "VpcLatticeAccessLogSubscription"
+    	AccessLogSubscriptionAnnotationKey = k8s.AnnotationPrefix + "accessLogSubscription"
     )
     // +genclient
@@ Expand Down @@