From 1ab78cb9e7c5ae50809b4b6ab97849345585037f Mon Sep 17 00:00:00 2001 From: Paulo Sousa Date: Mon, 12 Dec 2022 17:47:19 -0300 Subject: [PATCH] manager: add custom annotations as param on create/update instances --- internal/config/config.go | 2 ++ internal/config/config_test.go | 23 +++++++----- internal/pkg/rpaas/k8s.go | 10 ++++++ internal/pkg/rpaas/k8s_test.go | 57 ++++++++++++++++++++++++++++-- internal/pkg/rpaas/manager.go | 47 ++++++++++++++++++++++++ internal/pkg/rpaas/manager_test.go | 31 ++++++++++++++++ 6 files changed, 159 insertions(+), 11 deletions(-) diff --git a/internal/config/config.go b/internal/config/config.go index ecfdd1e51..831dee64f 100644 --- a/internal/config/config.go +++ b/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 { diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 0458ea553..8fa5d046b 100644 --- a/internal/config/config_test.go +++ b/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) diff --git a/internal/pkg/rpaas/k8s.go b/internal/pkg/rpaas/k8s.go index 1f6a60dc6..66ebeef4e 100644 --- a/internal/pkg/rpaas/k8s.go +++ b/internal/pkg/rpaas/k8s.go @@ -264,6 +264,7 @@ func (m *k8sRpaasManager) CreateInstance(ctx context.Context, args CreateArgs) e } setDescription(instance, args.Description) + setAnnotations(instance, args.Annotations()) instance.SetTeamOwner(args.Team) if m.clusterName != "" { instance.SetClusterName(m.clusterName) @@ -302,6 +303,7 @@ func (m *k8sRpaasManager) UpdateInstance(ctx context.Context, instanceName strin if m.clusterName != "" { instance.SetClusterName(m.clusterName) } + setAnnotations(instance, args.Annotations()) setTags(instance, args.Tags) setIP(instance, args.IP()) setLoadBalancerName(instance, args.LoadBalancerName()) @@ -1422,6 +1424,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 diff --git a/internal/pkg/rpaas/k8s_test.go b/internal/pkg/rpaas/k8s_test.go index f6d6d9349..77a73aeb4 100644 --- a/internal/pkg/rpaas/k8s_test.go +++ b/internal/pkg/rpaas/k8s_test.go @@ -3096,6 +3096,57 @@ 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\", \"rpaas.extensions.tsuru.io/tags\": \"tag1,tag2\", \"rpaas.extensions.tsuru.io/description\": \"my description\"}"}}, + 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", + }, + }, + }, + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -3148,7 +3199,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" @@ -3268,7 +3319,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\", \"rpaas.extensions.tsuru.io/tags\": \"tag1,tag2\", \"rpaas.extensions.tsuru.io/description\": \"my description\"}", }, }, assertion: func(t *testing.T, err error, instance *v1alpha1.RpaasInstance) { @@ -3278,6 +3330,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"]) diff --git a/internal/pkg/rpaas/manager.go b/internal/pkg/rpaas/manager.go index 55ebc2e0c..d2546ec83 100644 --- a/internal/pkg/rpaas/manager.go +++ b/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 { + 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 { + return getAnnotations(args.Parameters) +} + type PodStatusMap map[string]PodStatus type PodStatus struct { @@ -252,6 +262,43 @@ type CertificateData struct { Key string `json:"key"` } +func getAnnotations(params map[string]interface{}) (annotations map[string]string) { + p, found := params["annotations"] + if !found { + return + } + err := json.Unmarshal([]byte(p.(string)), &annotations) + if err != nil { + return + } + 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 { + delete(annotations, k) + continue + } + if errs := validation.IsValidLabelValue(annotationParts[1]); len(errs) != 0 { + delete(annotations, k) + continue + } + } else { + if errs := validation.IsValidLabelValue(annotationParts[0]); len(errs) != 0 { + delete(annotations, k) + continue + } + } + for _, prefix := range forbiddenAnnotationsPrefixes { + if strings.HasPrefix(k, prefix) { + delete(annotations, k) + } + } + } + return annotations +} + func getFlavors(params map[string]interface{}, tags []string) (flavors []string) { p, found := params["flavors"] if !found { diff --git a/internal/pkg/rpaas/manager_test.go b/internal/pkg/rpaas/manager_test.go index d45257b3a..8cb5413bb 100644 --- a/internal/pkg/rpaas/manager_test.go +++ b/internal/pkg/rpaas/manager_test.go @@ -8,6 +8,8 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/tsuru/rpaas-operator/internal/config" ) func TestCreateArgs_Flavors(t *testing.T) { @@ -319,3 +321,32 @@ func TestUpdateInstanceArgs_PlanOverride(t *testing.T) { }) } } +func TestUpdateInstanceArgs_Annotations(t *testing.T) { + tests := []struct { + args UpdateInstanceArgs + want map[string]string + }{ + {}, + { + args: UpdateInstanceArgs{ + Parameters: map[string]interface{}{"annotations": `{"image": "nginx:alpine", "rpaas.extensions.tsuru.io/test": "not valid", "test.io/valid": "valid"}`}, + }, + want: map[string]string{"image": "nginx:alpine", "test.io/valid": "valid"}, + }, + { + args: UpdateInstanceArgs{ + Parameters: map[string]interface{}{"annotations": `{"invalid_domain.io/test": "test"}`}, + }, + want: map[string]string{}, + }, + } + + for _, tt := range tests { + t.Run("", func(t *testing.T) { + err := config.Init() + require.NoError(t, err) + have := tt.args.Annotations() + assert.Equal(t, tt.want, have) + }) + } +}