PMM-12857 Fix API tests (part II) (#3106)

* PMM-12857 Fix annotation test * PMM-12857 shorten the generated name * PMM-12857 add a test to check for tag length * PMM-12857 remove the hostname from TestString
percona · Jul 31, 2024 · fb8105a · fb8105a
1 parent 439e2fc
commit fb8105a
Show file tree

Hide file tree

Showing 4 changed files with 47 additions and 13 deletions.
diff --git a/api-tests/helpers.go b/api-tests/helpers.go
@@ -56,10 +56,10 @@ func TestString(t TestingT, name string) string {
 	t.Helper()
 
 	// Without proper seed parallel tests can generate same "random" number.
-	n, err := rand.Int(rand.Reader, big.NewInt(math.MaxInt32))
+	rnd, err := rand.Int(rand.Reader, big.NewInt(math.MaxInt32))
 	require.NoError(t, err)
 
-	return strings.ReplaceAll(fmt.Sprintf("pmm-api-tests-%s-%s-%s-%d", Hostname, t.Name(), name, n), "/", "-")
+	return strings.ReplaceAll(fmt.Sprintf("api-test-%s-%s-%d", t.Name(), name, rnd), "/", "-")
 }
 
 // AssertAPIErrorf check that actual API error equals expected.

diff --git a/api-tests/management/annotation_test.go b/api-tests/management/annotation_test.go
@@ -16,6 +16,8 @@
 package management
 
 import (
+	"fmt"
+	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -55,7 +57,7 @@ func TestAddAnnotation(t *testing.T) {
 		pmmapitests.AssertAPIErrorf(t, err, 400, codes.InvalidArgument, "invalid AddAnnotationRequest.Text: value length must be at least 1 runes")
 	})
 
-	t.Run("Non-existing service", func(t *testing.T) {
+	t.Run("Non-existent service", func(t *testing.T) {
 		params := &mservice.AddAnnotationParams{
 			Body: mservice.AddAnnotationBody{
 				Text:         "Some text",
@@ -67,7 +69,7 @@ func TestAddAnnotation(t *testing.T) {
 		pmmapitests.AssertAPIErrorf(t, err, 404, codes.NotFound, `Service with name "no-service" not found.`)
 	})
 
-	t.Run("Non-existing node", func(t *testing.T) {
+	t.Run("Non-existent node", func(t *testing.T) {
 		params := &mservice.AddAnnotationParams{
 			Body: mservice.AddAnnotationBody{
 				Text:     "Some text",
@@ -79,6 +81,33 @@ func TestAddAnnotation(t *testing.T) {
 		pmmapitests.AssertAPIErrorf(t, err, 404, codes.NotFound, `Node with name "no-node" not found.`)
 	})
 
+	t.Run("Tag length exceeded", func(t *testing.T) {
+		nodeName := pmmapitests.TestString(t, strings.Repeat("long-annotation-node-name-", 10))
+		paramsNode := &nodes.AddNodeParams{
+			Body: nodes.AddNodeBody{
+				Generic: &nodes.AddNodeParamsBodyGeneric{
+					NodeName: nodeName,
+					Address:  "10.0.0.1",
+				},
+			},
+			Context: pmmapitests.Context,
+		}
+		resNode, err := inventoryClient.Default.NodesService.AddNode(paramsNode)
+		assert.NoError(t, err)
+		genericNodeID := resNode.Payload.Generic.NodeID
+		defer pmmapitests.RemoveNodes(t, genericNodeID)
+
+		params := &mservice.AddAnnotationParams{
+			Body: mservice.AddAnnotationBody{
+				Text:     "Some text",
+				NodeName: nodeName,
+			},
+			Context: pmmapitests.Context,
+		}
+		_, err = client.Default.ManagementService.AddAnnotation(params)
+		pmmapitests.AssertAPIErrorf(t, err, 400, codes.InvalidArgument, fmt.Sprintf("tag length cannot exceed 100 characters, tag: %s", nodeName))
+	})
+
 	t.Run("Existing service", func(t *testing.T) {
 		nodeName := pmmapitests.TestString(t, "annotation-node")
 		paramsNode := &nodes.AddNodeParams{
@@ -95,7 +124,7 @@ func TestAddAnnotation(t *testing.T) {
 		genericNodeID := resNode.Payload.Generic.NodeID
 		defer pmmapitests.RemoveNodes(t, genericNodeID)
 
-		serviceName := pmmapitests.TestString(t, "annotation-service")
+		serviceName := "annotation-service"
 		paramsService := &services.AddServiceParams{
 			Body: services.AddServiceBody{
 				Mysql: &services.AddServiceParamsBodyMysql{
@@ -126,7 +155,7 @@ func TestAddAnnotation(t *testing.T) {
 	})
 
 	t.Run("Existing node", func(t *testing.T) {
-		nodeName := pmmapitests.TestString(t, "annotation-node")
+		nodeName := "annotation-node"
 		params := &nodes.AddNodeParams{
 			Body: nodes.AddNodeBody{
 				Generic: &nodes.AddNodeParamsBodyGeneric{

diff --git a/api-tests/management/nodes_test.go b/api-tests/management/nodes_test.go
@@ -59,7 +59,7 @@ func TestNodeRegister(t *testing.T) {
 		})
 
 		t.Run("Reregister with same node name (no re-register - should fail)", func(t *testing.T) {
-			nodeName := pmmapitests.TestString(t, "node-name-for-all-fields")
+			nodeName := pmmapitests.TestString(t, "node-all")
 			nodeID, pmmAgentID := RegisterGenericNode(t, mservice.RegisterNodeBody{
 				NodeName: nodeName,
 				NodeType: pointer.ToString(mservice.RegisterNodeBodyNodeTypeNODETYPEGENERICNODE),
@@ -85,7 +85,7 @@ func TestNodeRegister(t *testing.T) {
 		})
 
 		t.Run("Reregister with same node name (re-register)", func(t *testing.T) {
-			nodeName := pmmapitests.TestString(t, "node-name-for-all-fields")
+			nodeName := pmmapitests.TestString(t, "node-all")
 			nodeID, pmmAgentID := RegisterGenericNode(t, mservice.RegisterNodeBody{
 				NodeName: nodeName,
 				NodeType: pointer.ToString(mservice.RegisterNodeBodyNodeTypeNODETYPEGENERICNODE),
@@ -116,7 +116,7 @@ func TestNodeRegister(t *testing.T) {
 		})
 
 		t.Run("Reregister with different node name (no re-register - should fail)", func(t *testing.T) {
-			nodeName := pmmapitests.TestString(t, "node-name-for-all-fields")
+			nodeName := pmmapitests.TestString(t, "node-all")
 			nodeID, pmmAgentID := RegisterGenericNode(t, mservice.RegisterNodeBody{
 				NodeName: nodeName,
 				NodeType: pointer.ToString(mservice.RegisterNodeBodyNodeTypeNODETYPEGENERICNODE),
@@ -142,7 +142,7 @@ func TestNodeRegister(t *testing.T) {
 		})
 
 		t.Run("Reregister with different node name (re-register)", func(t *testing.T) {
-			nodeName := pmmapitests.TestString(t, "node-name-for-all-fields")
+			nodeName := pmmapitests.TestString(t, "node-all")
 			nodeID, pmmAgentID := RegisterGenericNode(t, mservice.RegisterNodeBody{
 				NodeName: nodeName,
 				NodeType: pointer.ToString(mservice.RegisterNodeBodyNodeTypeNODETYPEGENERICNODE),

diff --git a/managed/services/management/annotation.go b/managed/services/management/annotation.go
@@ -29,9 +29,7 @@ import (
 	"github.com/percona/pmm/managed/models"
 )
 
-// AddAnnotation create annotation in grafana.
-//
-//nolint:unparam
+// AddAnnotation creates an annotation in grafana.
 func (s *ManagementService) AddAnnotation(ctx context.Context, req *managementv1.AddAnnotationRequest) (*managementv1.AddAnnotationResponse, error) {
 	headers, ok := metadata.FromIncomingContext(ctx)
 	if !ok {
@@ -70,6 +68,13 @@ func (s *ManagementService) AddAnnotation(ctx context.Context, req *managementv1
 		postfix = append(postfix, "Node Name: "+req.NodeName)
 	}
 
+	for _, tag := range tags {
+		if len(tag) > 100 {
+			msg := fmt.Sprintf("tag length cannot exceed 100 characters, tag: %s", tag)
+			return nil, status.Error(codes.InvalidArgument, msg)
+		}
+	}
+
 	if len(postfix) != 0 {
 		req.Text += " (" + strings.Join(postfix, ". ") + ")"
 	}