Add read-only support for ACK resources via annotation

apis/core/v1alpha1/annotations.go

@@ -75,4 +75,10 @@ const (
 	// resource manager will leave the AWS resource intact when the K8s resource
 	// is deleted.
 	AnnotationDeletionPolicy = AnnotationPrefix + "deletion-policy"
+	// AnnotationReadOnly is an annotation whose value is a boolean indicating
+	// whether the resource is read-only. If this annotation is set to true on a
+	// CR, that means the user is indicating to the ACK service controller that
+	// the resource is read-only and should not be created/patched/deleted by the
+	// ACK service controller.
+	AnnotationReadOnly = AnnotationPrefix + "read-only"
 )

pkg/errors/error.go

@@ -24,6 +24,9 @@ var (
 	// information that the resource being checked for existence was
 	// previously-created out of band from ACK
 	AdoptedResourceNotFound = fmt.Errorf("adopted resource not found")
+	// ReadOnlyResourceNotFound is like NotFound but provides the caller with
+	// information that the resource is on read-only mode and was not found
+	ReadOnlyResourceNotFound = fmt.Errorf("read-only resource not found")
 	// MissingNameIdentifier indicates an unexpected nil name identifier pointer
 	MissingNameIdentifier = fmt.Errorf("expected name identifier, found nil")
 	// NotAdoptable is to indicate the current resource has been explicitly

pkg/featuregate/features.go

@@ -17,6 +17,8 @@
 package featuregate
 
 const (
+	ReadOnly = "ReadOnly"
+
 	// TeamLevelCARM is a feature gate for enabling CARM for team-level resources.
 	TeamLevelCARM = "TeamLevelCARM"
 
@@ -28,6 +30,7 @@ const (
 // representing the default feature gates for ACK controllers.
 var defaultACKFeatureGates = FeatureGates{
 	// Set feature gates here
+	ReadOnly:         {Stage: Alpha, Enabled: false},
 	TeamLevelCARM:    {Stage: Alpha, Enabled: false},
 	ServiceLevelCARM: {Stage: Alpha, Enabled: false},
 }

pkg/runtime/reconciler.go

@@ -308,8 +308,12 @@ func (r *resourceReconciler) reconcile(
 	res acktypes.AWSResource,
 ) (acktypes.AWSResource, error) {
 	if res.IsBeingDeleted() {
-		// Determine whether we should retain or delete the resource
-		if r.getDeletionPolicy(res) == ackv1alpha1.DeletionPolicyDelete {
+		// We only delete resources that are not read-only and have a deletion
+		// policy set to delete.
+		if r.getDeletionPolicy(res) == ackv1alpha1.DeletionPolicyDelete &&
+			// If the ReadOnly feature gate is enabled, and the resource is read-only,
+			// we don't delete the resource.
+			!(r.cfg.FeatureGates.IsEnabled(featuregate.ReadOnly) && IsReadOnly(res)) {
 			// Resolve references before deleting the resource.
 			// Ignore any errors while resolving the references
 			resolved, _, _ := rm.ResolveReferences(ctx, r.apiReader, res)
@@ -356,6 +360,60 @@ func (r *resourceReconciler) Sync(
 	isAdopted := IsAdopted(desired)
 	rlog.WithValues("is_adopted", isAdopted)
 
+	if r.cfg.FeatureGates.IsEnabled(featuregate.ReadOnly) {
+		isReadOnly := IsReadOnly(desired)
+		rlog.WithValues("is_read_only", isReadOnly)
+
+		// NOTE(a-hilaly): When the time comes to support adopting resources
+		// using annotations, we will need to think a little bit more about
+		// the case where a user, wants to adopt a resource as read-only.
+
+		// If the resource is read-only, we enter a different code path where we
+		// only read the resource and patch the metadata and spec.
+		if isReadOnly {
+			rlog.Enter("rm.ResolveReferences")
+			resolved, _, _ := rm.ResolveReferences(ctx, r.apiReader, desired)
+			// NOTE(a-hilaly): One might be wondering why are we ignoring the
+			// error here. The reason is that the current implementation of
+			// ResolveReference doesn't take into consideration the read-only
+			// resources (and their fields).
+			//
+			// In a nutshell, `ResolveReferences` implementation was designed to
+			// be used for the create/update operations, and not for the read-only
+			// resources. This means that the implementation of `ResolveReferences`
+			// returns an error when any of the required references (for create)
+			// are not resolved. This is not helpful for read-only resources.
+			//
+			// We need to refactor the `ResolveReferences` implementation to
+			// be able to resolve read-only required fields. For example, EKS
+			// NodeGroups, only require the `ClusterName` field to be resolved.
+			// This is not the case for create/update operations where we need to
+			// resolve all the references (e.g. `ClusterName`, `NodeRoleArn`, etc.).
+			// See https://github.com/aws-controllers-k8s/eks-controller/blob/main/pkg/resource/nodegroup/references.go#L72-L114
+			//
+			// What's the worst that can happen? If we don't resolve the references
+			// for read-only resources, ReadOne call will fail with an error, causing
+			// the resource to be requeued.
+			//
+			// TODO(a-hilaly): Implement resolve references for read-only resources,
+			// maybe part of the DynamicReferences work.
+			rlog.Exit("rm.ResolveReferences", err)
+
+			rlog.Enter("rm.ReadOne")
+			latest, err = rm.ReadOne(ctx, resolved)
+			rlog.Exit("rm.ReadOne", err)
+			if err != nil {
+				if err == ackerr.NotFound {
+					return nil, ackerr.ReadOnlyResourceNotFound
+				}
+				return latest, err
+			}
+
+			err = r.patchResourceStatus(ctx, desired, latest)
+			return latest, err
+		}
+	}
+
 	rlog.Enter("rm.ResolveReferences")
 	resolved, hasReferences, err := rm.ResolveReferences(ctx, r.apiReader, desired)
 	rlog.Exit("rm.ResolveReferences", err)

pkg/runtime/reconciler_test.go

@@ -24,6 +24,7 @@ import (
 	"github.com/stretchr/testify/require"
 	"go.uber.org/zap/zapcore"
 	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	k8sobj "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	k8srtschema "k8s.io/apimachinery/pkg/runtime/schema"
@@ -198,6 +199,69 @@ func TestReconcilerCreate_BackoffRetries(t *testing.T) {
 	rm.AssertNumberOfCalls(t, "ReadOne", 6)
 }
 
+func TestReconcilerReadOnlyResource(t *testing.T) {
+	require := require.New(t)
+
+	ctx := context.TODO()
+	arn := ackv1alpha1.AWSResourceName("my-read-only-book-arn")
+
+	desired, _, metaObj := resourceMocks()
+	desired.On("ReplaceConditions", []*ackv1alpha1.Condition{}).Return()
+	metaObj.SetAnnotations(map[string]string{
+		ackv1alpha1.AnnotationReadOnly: "true",
+	})
+
+	ids := &ackmocks.AWSResourceIdentifiers{}
+	ids.On("ARN").Return(&arn)
+
+	latest, latestRTObj, _ := resourceMocks()
+	latest.On("Identifiers").Return(ids)
+	latest.On("Conditions").Return([]*ackv1alpha1.Condition{})
+	latest.On("MetaObject").Return(metav1.ObjectMeta{
+		Annotations: map[string]string{
+			ackv1alpha1.AnnotationReadOnly: "true",
+		},
+	})
+	latest.On("Conditions").Return([]*ackv1alpha1.Condition{})
+	latest.On(
+		"ReplaceConditions",
+		mock.AnythingOfType("[]*v1alpha1.Condition"),
+	).Return()
+
+	rm := &ackmocks.AWSResourceManager{}
+	rm.On("ResolveReferences", ctx, nil, desired).Return(
+		desired, false, nil,
+	).Times(2)
+	rm.On("ClearResolvedReferences", desired).Return(desired)
+	rm.On("ClearResolvedReferences", latest).Return(latest)
+	rm.On("ReadOne", ctx, desired).Return(
+		latest, nil,
+	).Once()
+	rm.On("Create", ctx, desired).Return(
+		latest, nil,
+	)
+	rm.On("IsSynced", ctx, latest).Return(true, nil)
+	rmf, rd := managedResourceManagerFactoryMocks(desired, latest)
+
+	rm.On("LateInitialize", ctx, latest).Return(latest, nil)
+	rd.On("IsManaged", desired).Return(true)
+	rd.On("Delta", desired, latest).Return(ackcompare.NewDelta())
+	rd.On("Delta", latest, latest).Return(ackcompare.NewDelta())
+
+	r, kc, scmd := reconcilerMocks(rmf)
+	rm.On("EnsureTags", ctx, desired, scmd).Return(nil)
+	statusWriter := &ctrlrtclientmock.SubResourceWriter{}
+	kc.On("Status").Return(statusWriter)
+	statusWriter.On("Patch", ctx, latestRTObj, mock.AnythingOfType("*client.mergeFromPatch")).Return(nil)
+	_, err := r.Sync(ctx, rm, desired)
+	require.Nil(err)
+	rm.AssertNumberOfCalls(t, "ReadOne", 1)
+	// Assert that the resource is not created or updated
+	rm.AssertNotCalled(t, "Create", 0)
+	rm.AssertNotCalled(t, "Update", 0)
+	rm.AssertNotCalled(t, "Delta", 0)
+}
+
 func TestReconcilerCreate_UnManagedResource_CheckReferencesResolveOnce(t *testing.T) {
 	require := require.New(t)
 

pkg/runtime/util.go

@@ -52,3 +52,19 @@ func IsSynced(res acktypes.AWSResource) bool {
 	}
 	return false
 }
+
+// IsReadOnly returns true if the supplied AWSResource has an annotation
+// indicating that it is in read-only mode.
+func IsReadOnly(res acktypes.AWSResource) bool {
+	mo := res.MetaObject()
+	if mo == nil {
+		// Should never happen... if it does, it's buggy code.
+		panic("IsReadOnly received resource with nil RuntimeObject")
+	}
+	for k, v := range mo.GetAnnotations() {
+		if k == ackv1alpha1.AnnotationReadOnly {
+			return strings.ToLower(v) == "true"
+		}
+	}
+	return false
+}