Make the annotation experimental...

... and complete comments

Signed-off-by: David Festal <dfestal@redhat.com>

pkg/apis/workload/v1alpha1/types.go

@@ -162,19 +162,30 @@ const (
 	// check all the APIBindings with this annotation for scheduling purpose.
 	ComputeAPIExportAnnotationKey = "extra.apis.kcp.io/compute.workload.kcp.io"
 
-	// UpsyncDerivedResourcesAnnotationKey is an annotation that can be set on a syncable resource.
+	// ExperimentalUpsyncDerivedResourcesAnnotationKey is an annotation that can be set on a syncable resource.
 	// It defines the resource types of derived resources (i.e. resources created from the syncable resource
 	// by some controller and that will not exist without it) intended to be upsynced to KCP.
 	//
 	// It contains a command-separated list of stringified GroupResource (<resource>.<group>).
 	//
 	// To allow upsyncing an Endpoints resource related to a synced service, the Service instance should be annotated with:
 	//
-	//   workload.kcp.io/upsync-derived-resources: endpoints
+	//   experimental.workload.kcp.io/upsync-derived-resources: endpoints
 	//
-	// Of course this also requires having, on the physical cluster, the appropriate logic that will effectively
-	// label the derived resources for Upsync.
 	// For now, only endpoints can be upsynced on demand by the syncer with this mechanism,
-	// but the list of such resources will increase in the future.
-	UpsyncDerivedResourcesAnnotationKey = "workload.kcp.io/upsync-derived-resources"
+	// but the list of such resources would increase in the future.
+	//
+	// Of course using this annotation also requires having, on the physical cluster, the appropriate logic
+	// that will effectively label the derived resources for Upsync.
+	// This logic should guard against upsyncing unexpected resources.
+	// In addition, Upsyncing is limited to a limited, well-defined list of resource types on the KCP side,
+	// so that simply adding this annotation on a synced resource will not be a security risk.
+	//
+	// This annotation is user-facing and would typically be set by the client creating the synced resource
+	// in the KCP workspace, be it the end-user or a third-party controller.
+	//
+	// It is experimental since the provided user-experience is unsatisfactory,
+	// and further work should be done to define such (up)syncing strategies at a more appropriate level
+	// (SyncTarget, KCP namespace, KCP workspace ?).
+	ExperimentalUpsyncDerivedResourcesAnnotationKey = "experimental.workload.kcp.io/upsync-derived-resources"
 )

pkg/syncer/endpoints/endpoint_downstream_process.go

@@ -90,7 +90,7 @@ func (c *controller) process(ctx context.Context, key string) error {
 		return nil
 	}
 
-	derivedResourcesToUpsync := strings.Split(service.GetAnnotations()[workloadv1alpha1.UpsyncDerivedResourcesAnnotationKey], ",")
+	derivedResourcesToUpsync := strings.Split(service.GetAnnotations()[workloadv1alpha1.ExperimentalUpsyncDerivedResourcesAnnotationKey], ",")
 	if len(derivedResourcesToUpsync) == 0 ||
 		!sets.NewString(derivedResourcesToUpsync...).Has(endpointsGVR.GroupResource().String()) {
 		logger.V(3).Info("ignoring endpoint since it is not mentioned in the service 'workload.kcp.io/upsync-derived-resources' annotation")

pkg/syncer/endpoints/endpoint_downstream_process_test.go

@@ -79,7 +79,7 @@ func TestEndpointsControllerProcess(t *testing.T) {
 			}).Object(),
 			downstreamEndpoints: endpoints("httpecho").WithNamespace("downstream-ns").Object(),
 			downstreamService: service("httpecho").WithNamespace("downstream-ns").WithAnnotations(map[string]string{
-				"workload.kcp.io/upsync-derived-resources": "pods,endpoints",
+				"experimental.workload.kcp.io/upsync-derived-resources": "pods,endpoints",
 			}).Object(),
 			expectedPatch:     `{"metadata": {"labels": {"state.workload.kcp.io/6ohB8yeXhwqTQVuBzJRgqcRJTpRjX7yTZu5g5g":"Upsync"}}}`,
 			expectedPatchType: types.StrategicMergePatchType,

test/e2e/syncer/endpoints/service-with-upsync.yaml

@@ -4,7 +4,7 @@ metadata:
   name: with-endpoints-upsync
   namespace: default
   annotations:
-    workload.kcp.io/upsync-derived-resources: endpoints
+    experimental.workload.kcp.io/upsync-derived-resources: endpoints
 spec:
   selector:
     app.kubernetes.io/name: with-endpoints-upsync