manager: add custom annotations as param on create/update instances

internal/config/config.go

@@ -53,6 +53,7 @@ type RpaasConfig struct {
 	NamespacedInstances                  bool                       `json:"namespaced-instances"`
 	EnableCertManager                    bool                       `json:"enable-cert-manager"`
 	NewInstanceReplicas                  int                        `json:"new-instance-replicas"`
+	ForbiddenAnnotationsPrefixes         []string                   `json:"forbidden-annotations-prefixes"`
 }
 
 type ClusterConfig struct {
@@ -109,6 +110,7 @@ func Init() error {
 	viper.SetDefault("websocket-write-wait", time.Second)
 	viper.SetDefault("enable-cert-manager", false)
 	viper.SetDefault("new-instance-replicas", 1)
+	viper.SetDefault("forbidden-annotations-prefixes", []string{"rpaas.extensions.tsuru.io", "afh.tsuru.io"})
 	viper.AutomaticEnv()
 	err := readConfig()
 	if err != nil {

internal/config/config_test.go

@@ -175,6 +175,9 @@ websocket-allowed-origins:
 config-deny-patterns:
 - pattern1.*
 - pattern2.*
+forbidden-annotations-prefixes:
+- foo.bar/test
+- foo.bar/another
 `,
 			expected: func(c RpaasConfig) RpaasConfig {
 				c.WebSocketHandshakeTimeout = time.Minute
@@ -188,6 +191,7 @@ config-deny-patterns:
 					*regexp.MustCompile(`pattern1.*`),
 					*regexp.MustCompile(`pattern2.*`),
 				}
+				c.ForbiddenAnnotationsPrefixes = []string{"foo.bar/test", "foo.bar/another"}
 				return c
 			},
 		},
@@ -211,15 +215,16 @@ config-deny-patterns:
 			require.NoError(t, err)
 			config := Get()
 			expected := RpaasConfig{
-				ServiceName:               "rpaasv2",
-				SyncInterval:              5 * time.Minute,
-				WebSocketHandshakeTimeout: 5 * time.Second,
-				WebSocketReadBufferSize:   1024,
-				WebSocketWriteBufferSize:  4096,
-				WebSocketPingInterval:     2 * time.Second,
-				WebSocketMaxIdleTime:      1 * time.Minute,
-				WebSocketWriteWait:        time.Second,
-				NewInstanceReplicas:       1,
+				ServiceName:                  "rpaasv2",
+				SyncInterval:                 5 * time.Minute,
+				WebSocketHandshakeTimeout:    5 * time.Second,
+				WebSocketReadBufferSize:      1024,
+				WebSocketWriteBufferSize:     4096,
+				WebSocketPingInterval:        2 * time.Second,
+				WebSocketMaxIdleTime:         1 * time.Minute,
+				WebSocketWriteWait:           time.Second,
+				NewInstanceReplicas:          1,
+				ForbiddenAnnotationsPrefixes: []string{"rpaas.extensions.tsuru.io", "afh.tsuru.io"},
 			}
 			if tt.expected != nil {
 				expected = tt.expected(expected)

internal/pkg/rpaas/k8s.go

@@ -264,6 +264,11 @@ func (m *k8sRpaasManager) CreateInstance(ctx context.Context, args CreateArgs) e
 	}
 
 	setDescription(instance, args.Description)
+	annotations, err := args.Annotations()
+	if err != nil {
+		return err
+	}
+	setAnnotations(instance, annotations)
 	instance.SetTeamOwner(args.Team)
 	if m.clusterName != "" {
 		instance.SetClusterName(m.clusterName)
@@ -280,12 +285,13 @@ func (m *k8sRpaasManager) CreateInstance(ctx context.Context, args CreateArgs) e
 }
 
 func (m *k8sRpaasManager) UpdateInstance(ctx context.Context, instanceName string, args UpdateInstanceArgs) error {
+	var err error
 	instance, err := m.GetInstance(ctx, instanceName)
 	if err != nil {
 		return err
 	}
 
-	if err := m.validateUpdateInstanceArgs(ctx, instance, args); err != nil {
+	if err = m.validateUpdateInstanceArgs(ctx, instance, args); err != nil {
 		return err
 	}
 
@@ -302,6 +308,11 @@ func (m *k8sRpaasManager) UpdateInstance(ctx context.Context, instanceName strin
 	if m.clusterName != "" {
 		instance.SetClusterName(m.clusterName)
 	}
+	annotations, err := args.Annotations()
+	if err != nil {
+		return err
+	}
+	setAnnotations(instance, annotations)
 	setTags(instance, args.Tags)
 	setIP(instance, args.IP())
 	setLoadBalancerName(instance, args.LoadBalancerName())
@@ -1422,6 +1433,14 @@ func mergeMap(a, b map[string]string) map[string]string {
 	return a
 }
 
+func setAnnotations(instance *v1alpha1.RpaasInstance, annotations map[string]string) {
+	if instance == nil {
+		return
+	}
+
+	instance.Annotations = mergeMap(instance.Annotations, annotations)
+}
+
 func setDescription(instance *v1alpha1.RpaasInstance, description string) {
 	if instance == nil {
 		return

internal/pkg/rpaas/k8s_test.go

@@ -7,6 +7,7 @@ package rpaas
 import (
 	"context"
 	"crypto/tls"
+	"fmt"
 	"net/http"
 	"regexp"
 	"testing"
@@ -23,6 +24,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
+	"k8s.io/apimachinery/pkg/util/validation"
 	metricsv1beta1 "k8s.io/metrics/pkg/apis/metrics/v1beta1"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
@@ -3096,6 +3098,63 @@ func Test_k8sRpaasManager_CreateInstance(t *testing.T) {
 				},
 			},
 		},
+		{
+			name:        "with custom annotations only set allowed ones",
+			args:        CreateArgs{Name: "r1", Team: "t1", Parameters: map[string]interface{}{"annotations": "{\"my-custom-annotation\": \"my-value\"}"}},
+			extraConfig: config.RpaasConfig{ForbiddenAnnotationsPrefixes: []string{"rpaas.extensions.tsuru.io"}},
+			expected: v1alpha1.RpaasInstance{
+				TypeMeta: metav1.TypeMeta{
+					Kind:       "RpaasInstance",
+					APIVersion: "extensions.tsuru.io/v1alpha1",
+				},
+				ObjectMeta: metav1.ObjectMeta{
+					Name:            "r1",
+					Namespace:       "rpaasv2",
+					ResourceVersion: "1",
+					Annotations: map[string]string{
+						"rpaas.extensions.tsuru.io/tags":        "",
+						"rpaas.extensions.tsuru.io/description": "",
+						"rpaas.extensions.tsuru.io/team-owner":  "t1",
+						"my-custom-annotation":                  "my-value",
+					},
+					Labels: map[string]string{
+						"rpaas.extensions.tsuru.io/service-name":  "rpaasv2",
+						"rpaas.extensions.tsuru.io/instance-name": "r1",
+						"rpaas.extensions.tsuru.io/team-owner":    "t1",
+						"rpaas_service":                           "rpaasv2",
+						"rpaas_instance":                          "r1",
+					},
+				},
+				Spec: v1alpha1.RpaasInstanceSpec{
+					Replicas: &one,
+					PlanName: "plan1",
+					Service: &nginxv1alpha1.NginxService{
+						Labels: map[string]string{
+							"rpaas.extensions.tsuru.io/service-name":  "rpaasv2",
+							"rpaas.extensions.tsuru.io/instance-name": "r1",
+							"rpaas.extensions.tsuru.io/team-owner":    "t1",
+							"rpaas_service":                           "rpaasv2",
+							"rpaas_instance":                          "r1",
+						},
+					},
+					PodTemplate: nginxv1alpha1.NginxPodTemplateSpec{
+						Labels: map[string]string{
+							"rpaas.extensions.tsuru.io/service-name":  "rpaasv2",
+							"rpaas.extensions.tsuru.io/instance-name": "r1",
+							"rpaas.extensions.tsuru.io/team-owner":    "t1",
+							"rpaas_service":                           "rpaasv2",
+							"rpaas_instance":                          "r1",
+						},
+					},
+				},
+			},
+		},
+		{
+			name:          "with custom annotations and forbidden prefixes",
+			args:          CreateArgs{Name: "r1", Team: "t1", Parameters: map[string]interface{}{"annotations": "{\"my-custom-annotation\": \"my-value\", \"rpaas.extensions.tsuru.io/tags\": \"tag1,tag2\", \"rpaas.extensions.tsuru.io/description\": \"my description\"}"}},
+			extraConfig:   config.RpaasConfig{ForbiddenAnnotationsPrefixes: []string{"rpaas.extensions.tsuru.io"}},
+			expectedError: `annotation "rpaas.extensions.tsuru.io/tags" is not allowed`,
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -3148,7 +3207,7 @@ func Test_k8sRpaasManager_CreateInstance(t *testing.T) {
 func Test_k8sRpaasManager_UpdateInstance(t *testing.T) {
 	cfg := config.Get()
 	defer func() { config.Set(cfg) }()
-	config.Set(config.RpaasConfig{LoadBalancerNameLabelKey: "cloudprovider.example/lb-name"})
+	config.Set(config.RpaasConfig{LoadBalancerNameLabelKey: "cloudprovider.example/lb-name", ForbiddenAnnotationsPrefixes: []string{"rpaas.extensions.tsuru.io"}})
 
 	instance1 := newEmptyRpaasInstance()
 	instance1.Name = "instance1"
@@ -3259,6 +3318,23 @@ func Test_k8sRpaasManager_UpdateInstance(t *testing.T) {
 				assert.Equal(t, `cannot unset flavor "feature-create-only" as it is a creation only flavor`, err.Error())
 			},
 		},
+		{
+			name:     "when tries to set an invalid annotation format",
+			instance: "instance1",
+			args: UpdateInstanceArgs{
+				Description: "Another description",
+				Plan:        "plan2",
+				Tags:        []string{},
+				Team:        "team-two",
+				Parameters: map[string]interface{}{
+					"annotations": "{\"test.io/_my-custom-annotation\": \"my-value\"}",
+				},
+			},
+			assertion: func(t *testing.T, err error, instance *v1alpha1.RpaasInstance) {
+				require.Error(t, err)
+				assert.Equal(t, fmt.Sprintf(`invalid annotation "test.io/_my-custom-annotation": %v`, validation.IsValidLabelValue("test.io/_my-custom-annotation")), err.Error())
+			},
+		},
 		{
 			name:     "when successfully updating an instance",
 			instance: "instance1",
@@ -3268,7 +3344,8 @@ func Test_k8sRpaasManager_UpdateInstance(t *testing.T) {
 				Tags:        []string{"tag3", "tag4", "tag5", `plan-override={"image": "my.registry.test/nginx:latest"}`},
 				Team:        "team-two",
 				Parameters: map[string]interface{}{
-					"lb-name": "my-instance.example",
+					"lb-name":     "my-instance.example",
+					"annotations": "{\"my-custom-annotation\": \"my-value\"}",
 				},
 			},
 			assertion: func(t *testing.T, err error, instance *v1alpha1.RpaasInstance) {
@@ -3278,6 +3355,7 @@ func Test_k8sRpaasManager_UpdateInstance(t *testing.T) {
 				require.NotNil(t, instance.Labels)
 				assert.Equal(t, "team-two", instance.Labels["rpaas.extensions.tsuru.io/team-owner"])
 				require.NotNil(t, instance.Annotations)
+				assert.Equal(t, "my-value", instance.Annotations["my-custom-annotation"])
 				assert.Equal(t, "Another description", instance.Annotations["rpaas.extensions.tsuru.io/description"])
 				assert.Equal(t, `plan-override={"image": "my.registry.test/nginx:latest"},tag3,tag4,tag5`, instance.Annotations["rpaas.extensions.tsuru.io/tags"])
 				assert.Equal(t, "team-two", instance.Annotations["rpaas.extensions.tsuru.io/team-owner"])

internal/pkg/rpaas/manager.go

@@ -17,9 +17,11 @@ import (
 	"text/template"
 
 	nginxv1alpha1 "github.com/tsuru/nginx-operator/api/v1alpha1"
+	"k8s.io/apimachinery/pkg/util/validation"
 	osb "sigs.k8s.io/go-open-service-broker-client/v2"
 
 	"github.com/tsuru/rpaas-operator/api/v1alpha1"
+	"github.com/tsuru/rpaas-operator/internal/config"
 	clientTypes "github.com/tsuru/rpaas-operator/pkg/rpaas/client/types"
 )
 
@@ -110,6 +112,10 @@ func (args CreateArgs) PlanOverride() string {
 	return getPlanOverride(args.Parameters, args.Tags)
 }
 
+func (args CreateArgs) Annotations() (map[string]string, error) {
+	return getAnnotations(args.Parameters)
+}
+
 type UpdateInstanceArgs struct {
 	Team        string                 `form:"team"`
 	Description string                 `form:"description"`
@@ -134,6 +140,10 @@ func (args UpdateInstanceArgs) PlanOverride() string {
 	return getPlanOverride(args.Parameters, args.Tags)
 }
 
+func (args UpdateInstanceArgs) Annotations() (map[string]string, error) {
+	return getAnnotations(args.Parameters)
+}
+
 type PodStatusMap map[string]PodStatus
 
 type PodStatus struct {
@@ -252,6 +262,45 @@ type CertificateData struct {
 	Key         string `json:"key"`
 }
 
+func getAnnotations(params map[string]interface{}) (map[string]string, error) {
+	p, found := params["annotations"]
+	if !found {
+		return nil, nil
+	}
+	annotationsStr, ok := p.(string)
+	if !ok {
+		return nil, fmt.Errorf("invalid annotations: %v", p)
+	}
+	annotations := map[string]string{}
+	err := json.Unmarshal([]byte(annotationsStr), &annotations)
+	if err != nil {
+		return nil, err
+	}
+	forbiddenAnnotationsPrefixes := config.Get().ForbiddenAnnotationsPrefixes
+	for k := range annotations {
+		// check if annotation complies with allowed k8s annotations
+		annotationParts := strings.Split(k, "/")
+		if len(annotationParts) > 1 {
+			if errs := validation.IsDNS1123Subdomain(annotationParts[0]); len(errs) != 0 {
+				return nil, fmt.Errorf("invalid annotation %q: %s", k, errs)
+			}
+			if errs := validation.IsValidLabelValue(annotationParts[1]); len(errs) != 0 {
+				return nil, fmt.Errorf("invalid annotation %q: %s", k, errs)
+			}
+		} else {
+			if errs := validation.IsValidLabelValue(annotationParts[0]); len(errs) != 0 {
+				return nil, fmt.Errorf("invalid annotation %q: %s", k, errs)
+			}
+		}
+		for _, prefix := range forbiddenAnnotationsPrefixes {
+			if strings.HasPrefix(k, prefix) {
+				return nil, fmt.Errorf("annotation %q is not allowed", k)
+			}
+		}
+	}
+	return annotations, nil
+}
+
 func getFlavors(params map[string]interface{}, tags []string) (flavors []string) {
 	p, found := params["flavors"]
 	if !found {

internal/pkg/rpaas/manager_test.go

@@ -5,9 +5,13 @@
 package rpaas
 
 import (
+	"fmt"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"github.com/tsuru/rpaas-operator/internal/config"
+	"k8s.io/apimachinery/pkg/util/validation"
 )
 
 func TestCreateArgs_Flavors(t *testing.T) {
@@ -319,3 +323,43 @@ func TestUpdateInstanceArgs_PlanOverride(t *testing.T) {
 		})
 	}
 }
+func TestUpdateInstanceArgs_Annotations(t *testing.T) {
+	tests := []struct {
+		args          UpdateInstanceArgs
+		want          map[string]string
+		expectedError string
+	}{
+		{},
+		{
+			args: UpdateInstanceArgs{
+				Parameters: map[string]interface{}{"annotations": `{"image": "nginx:alpine", "test.io/valid": "valid"}`},
+			},
+			want: map[string]string{"image": "nginx:alpine", "test.io/valid": "valid"},
+		},
+		{
+			args: UpdateInstanceArgs{
+				Parameters: map[string]interface{}{"annotations": `{"image": "nginx:alpine", "rpaas.extensions.tsuru.io/test": "not valid", "test.io/valid": "valid"}`},
+			},
+			expectedError: "annotation \"rpaas.extensions.tsuru.io/test\" is not allowed",
+		},
+		{
+			args: UpdateInstanceArgs{
+				Parameters: map[string]interface{}{"annotations": `{"invalid_domain.io/test": "test"}`},
+			},
+			expectedError: fmt.Sprintf("invalid annotation \"invalid_domain.io/test\": %s", validation.IsDNS1123Subdomain("invalid_domain.io/test")),
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run("", func(t *testing.T) {
+			err := config.Init()
+			require.NoError(t, err)
+			have, err := tt.args.Annotations()
+			if tt.expectedError != "" {
+				assert.EqualError(t, err, tt.expectedError)
+				return
+			}
+			assert.Equal(t, tt.want, have)
+		})
+	}
+}