From fa3e23a38179034730b43b40c43ada3e64c15c39 Mon Sep 17 00:00:00 2001 From: Bartosz Leper Date: Wed, 18 Sep 2024 20:03:56 +0200 Subject: [PATCH] [v16] Add a link text label to the ClusterAlert resource (#46284) * Add an alert CTA label This label allows to customize text on a button that contains the alert link. * Comment * Update api/types/cluster_alert.go Co-authored-by: Zac Bergquist * Rename CTA to "link text" --------- Co-authored-by: Zac Bergquist --- api/types/cluster_alert.go | 19 ++++++--- api/types/cluster_alert_test.go | 39 ++++++++++++++----- api/types/constants.go | 4 ++ lib/auth/auth.go | 8 ++-- .../teleport/aggregating/submitter.go | 2 + 5 files changed, 55 insertions(+), 17 deletions(-) diff --git a/api/types/cluster_alert.go b/api/types/cluster_alert.go index e98ee46af238..e5f8b0e3eeb9 100644 --- a/api/types/cluster_alert.go +++ b/api/types/cluster_alert.go @@ -32,6 +32,9 @@ var matchAlertLabelKey = regexp.MustCompile(`^[a-z0-9\.\-\/]+$`).MatchString // matchAlertLabelVal is a slightly more permissive matcher for label values. var matchAlertLabelVal = regexp.MustCompile(`^[a-z0-9\.\-_\/:|]+$`).MatchString +// matchAlertLabelLinkTextVal only allows alphanumeric characters and spaces. +var matchAlertLabelLinkTextVal = regexp.MustCompile(`^[a-zA-Z0-9 ]+$`).MatchString + const validLinkDestination = "goteleport.com" type alertOptions struct { @@ -151,12 +154,9 @@ func (c *ClusterAlert) CheckAndSetDefaults() error { if !matchAlertLabelKey(key) { return trace.BadParameter("invalid alert label key: %q", key) } - // for links, we relax the conditions on label values - if key != AlertLink && !matchAlertLabelVal(val) { - return trace.BadParameter("invalid alert label value: %q", val) - } - if key == AlertLink { + switch key { + case AlertLink: u, err := url.Parse(val) if err != nil { return trace.BadParameter("invalid alert: label link %q is not a valid URL", val) @@ -164,6 +164,15 @@ func (c *ClusterAlert) CheckAndSetDefaults() error { if u.Hostname() != validLinkDestination { return trace.BadParameter("invalid alert: label link not allowed %q", val) } + case AlertLinkText: + if !matchAlertLabelLinkTextVal(val) { + return trace.BadParameter("invalid alert: label button text not allowed: %q", val) + } + default: + if !matchAlertLabelVal(val) { + // for links, we relax the conditions on label values + return trace.BadParameter("invalid alert label value: %q", val) + } } } return nil diff --git a/api/types/cluster_alert_test.go b/api/types/cluster_alert_test.go index cba58d301239..b8b76dfdf879 100644 --- a/api/types/cluster_alert_test.go +++ b/api/types/cluster_alert_test.go @@ -84,32 +84,53 @@ func TestAlertSorting(t *testing.T) { } } -// TestCheckAndSetDefaults verifies that only valid URLs are set on the link label. +// TestCheckAndSetDefaults verifies that only valid URLs are set on the link +// label and that only valid link text can be used. func TestCheckAndSetDefaultsWithLink(t *testing.T) { tests := []struct { - link string - assert require.ErrorAssertionFunc + options []AlertOption + name string + assert require.ErrorAssertionFunc }{ { - link: "https://goteleport.com/docs", + name: "valid link", + options: []AlertOption{WithAlertLabel(AlertLink, "https://goteleport.com/docs")}, + assert: require.NoError, + }, + { + name: "valid link with link text", + options: []AlertOption{ + WithAlertLabel(AlertLink, "https://goteleport.com/support"), + WithAlertLabel(AlertLinkText, "Contact Support"), + }, assert: require.NoError, }, { - link: "h{t}tps://goteleport.com/docs", - assert: require.Error, + name: "invalid link", + options: []AlertOption{WithAlertLabel(AlertLink, "h{t}tps://goteleport.com/docs")}, + assert: require.Error, }, { - link: "https://google.com", + name: "external link", + options: []AlertOption{WithAlertLabel(AlertLink, "https://google.com")}, + assert: require.Error, + }, + { + name: "valid link with invalid link text", + options: []AlertOption{ + WithAlertLabel(AlertLink, "https://goteleport.com/support"), + WithAlertLabel(AlertLinkText, "Contact!Support"), + }, assert: require.Error, }, } for i, tt := range tests { - t.Run(tt.link, func(t *testing.T) { + t.Run(tt.name, func(t *testing.T) { _, err := NewClusterAlert( fmt.Sprintf("name-%d", i), fmt.Sprintf("message-%d", i), - WithAlertLabel(AlertLink, tt.link), + tt.options..., ) tt.assert(t, err) }) diff --git a/api/types/constants.go b/api/types/constants.go index 5afc1b0c4d82..0b0e809e66cf 100644 --- a/api/types/constants.go +++ b/api/types/constants.go @@ -960,6 +960,10 @@ const ( // AlertLink is an internal label that indicates that an alert is a link. AlertLink = TeleportInternalLabelPrefix + "link" + // AlertLinkText is a text that will be rendered by Web UI on the action + // button accompanying the alert. + AlertLinkText = TeleportInternalLabelPrefix + "link-text" + // AlertVerbPermit is an internal label that permits a user to view the alert if they // hold a specific resource permission verb (e.g. 'node:list'). Note that this label is // a coarser control than it might initially appear and has the potential for accidental diff --git a/lib/auth/auth.go b/lib/auth/auth.go index 02580f9f0263..f05dcdbaff8f 100644 --- a/lib/auth/auth.go +++ b/lib/auth/auth.go @@ -144,8 +144,9 @@ const ( OSSDesktopsAlertMessage = "Your cluster is beyond its allocation of 5 non-Active Directory Windows desktops. " + "Reach out for unlimited desktops with Teleport Enterprise." - OSSDesktopAlertLink = "https://goteleport.com/r/upgrade-community?utm_campaign=CTA_windows_local" - OSSDesktopsLimit = 5 + OSSDesktopsAlertLink = "https://goteleport.com/r/upgrade-community?utm_campaign=CTA_windows_local" + OSSDesktopsAlertLinkText = "Contact Sales" + OSSDesktopsLimit = 5 ) const ( @@ -5572,7 +5573,8 @@ func (a *Server) syncDesktopsLimitAlert(ctx context.Context) { types.WithAlertSeverity(types.AlertSeverity_MEDIUM), types.WithAlertLabel(types.AlertOnLogin, "yes"), types.WithAlertLabel(types.AlertPermitAll, "yes"), - types.WithAlertLabel(types.AlertLink, OSSDesktopAlertLink), + types.WithAlertLabel(types.AlertLink, OSSDesktopsAlertLink), + types.WithAlertLabel(types.AlertLinkText, OSSDesktopsAlertLinkText), types.WithAlertExpires(time.Now().Add(OSSDesktopsCheckPeriod))) if err != nil { log.Warnf("Can't create OSS non-AD desktops limit alert: %v", err) diff --git a/lib/usagereporter/teleport/aggregating/submitter.go b/lib/usagereporter/teleport/aggregating/submitter.go index 19a81a9a890b..78939e83d897 100644 --- a/lib/usagereporter/teleport/aggregating/submitter.go +++ b/lib/usagereporter/teleport/aggregating/submitter.go @@ -51,6 +51,7 @@ const ( alertGraceDuration = alertGraceHours * time.Hour alertName = "reporting-failed" alertLink = "https://goteleport.com/support/" + alertLinkText = "Contact Support" ) const ( @@ -204,6 +205,7 @@ func submitOnce(ctx context.Context, c SubmitterConfig) { types.WithAlertLabel(types.AlertOnLogin, "yes"), types.WithAlertLabel(types.AlertPermitAll, "yes"), types.WithAlertLabel(types.AlertLink, alertLink), + types.WithAlertLabel(types.AlertLinkText, alertLinkText), ) if err != nil { c.Log.WithError(err).Errorf("Failed to create cluster alert %v.", alertName)