MM-29264 Fix invalid server version handling / date constraint in InProduct Notices (#15756)

app/product_notices.go

@@ -6,6 +6,8 @@ package app
 import (
 	"net/http"
 	"reflect"
+	"regexp"
+	"strconv"
 	"strings"
 	"time"
 
@@ -32,6 +34,21 @@ var cachedUserCount int64
 
 // previously fetched notices
 var cachedNotices model.ProductNotices
+var rcStripRegexp = regexp.MustCompile(`(.*?)(-rc\d+)(.*?)`)
+
+func cleanupVersion(originalVersion string) string {
+	// clean up BuildNumber to remove release- prefix, -rc suffix and a hash part of the version
+	version := strings.Replace(originalVersion, "release-", "", 1)
+	version = rcStripRegexp.ReplaceAllString(version, `$1$3`)
+	versionParts := strings.Split(version, ".")
+	var versionPartsOut []string
+	for _, part := range versionParts {
+		if _, err := strconv.ParseInt(part, 10, 16); err == nil {
+			versionPartsOut = append(versionPartsOut, part)
+		}
+	}
+	return strings.Join(versionPartsOut, ".")
+}
 
 func noticeMatchesConditions(config *model.Config, preferences store.PreferenceStore, userId string, client model.NoticeClientType, clientVersion, locale string, postCount, userCount int64, isSystemAdmin, isTeamAdmin bool, isCloud bool, sku string, notice *model.ProductNotice) (bool, error) {
 	cnd := notice.Conditions
@@ -66,7 +83,7 @@ func noticeMatchesConditions(config *model.Config, preferences store.PreferenceS
 
 	// check if notice date range matches current
 	if cnd.DisplayDate != nil {
-		now := time.Now().UTC()
+		now := time.Now().UTC().Truncate(time.Hour * 24)
 		c, err2 := date_constraints.NewConstraint(*cnd.DisplayDate)
 		if err2 != nil {
 			return false, errors.Wrapf(err2, "Cannot parse date range %s", *cnd.DisplayDate)
@@ -77,10 +94,13 @@ func noticeMatchesConditions(config *model.Config, preferences store.PreferenceS
 	}
 
 	// check if current server version is notice range
-	serverVersion, err := semver.NewVersion(model.BuildNumber)
+	if cnd.ServerVersion != nil {
-	if err != nil {
+		version := cleanupVersion(model.BuildNumber)
-		mlog.Warn("Skipping server version check, build number is not in semver format", mlog.String("build_number", model.BuildNumber))
+		serverVersion, err := semver.NewVersion(version)
-	} else {
+		if err != nil {
+			mlog.Warn("Build number is not in semver format", mlog.String("build_number", version))
+			return false, nil
+		}
 		for _, v := range cnd.ServerVersion {
 			c, err := semver.NewConstraint(v)
 			if err != nil {

app/product_notices_test.go

@@ -12,6 +12,7 @@ import (
 	"net/http/httptest"
 	"strings"
 	"testing"
+	"time"
 )
 
 func TestNoticeValidation(t *testing.T) {
@@ -196,6 +197,77 @@ func TestNoticeValidation(t *testing.T) {
 			wantErr: false,
 			wantOk:  true,
 		},
+
+		{
+			name: "notice with server version check that matches a const",
+			args: args{
+				serverVersion: "99.1.1",
+				notice: &model.ProductNotice{
+					Conditions: model.Conditions{
+						ServerVersion: []string{"> 99.0.0"},
+					},
+				},
+			},
+			wantErr: false,
+			wantOk:  true,
+		},
+
+		{
+			name: "notice with server version check that has rc",
+			args: args{
+				serverVersion: "99.1.1-rc2",
+				notice: &model.ProductNotice{
+					Conditions: model.Conditions{
+						ServerVersion: []string{"> 99.0.0 < 100.2.2"},
+					},
+				},
+			},
+			wantErr: false,
+			wantOk:  true,
+		},
+
+		{
+			name: "notice with server version check that has rc and hash",
+			args: args{
+				serverVersion: "99.1.1-rc2.abcdef",
+				notice: &model.ProductNotice{
+					Conditions: model.Conditions{
+						ServerVersion: []string{"> 99.0.0 < 100.2.2"},
+					},
+				},
+			},
+			wantErr: false,
+			wantOk:  true,
+		},
+
+		{
+			name: "notice with server version check that has release and hash",
+			args: args{
+				serverVersion: "release-99.1.1.abcdef",
+				notice: &model.ProductNotice{
+					Conditions: model.Conditions{
+						ServerVersion: []string{"> 99.0.0 < 100.2.2"},
+					},
+				},
+			},
+			wantErr: false,
+			wantOk:  true,
+		},
+
+		{
+			name: "notice with server version check that has cloud version",
+			args: args{
+				serverVersion: "cloud.54.abcdef",
+				notice: &model.ProductNotice{
+					Conditions: model.Conditions{
+						ServerVersion: []string{"> 99.0.0 < 100.2.2"},
+					},
+				},
+			},
+			wantErr: false,
+			wantOk:  false,
+		},
+
 		{
 			name: "notice with server version check that is invalid",
 			args: args{
@@ -248,7 +320,18 @@ func TestNoticeValidation(t *testing.T) {
 			wantErr: false,
 			wantOk:  true,
 		},
-
+		{
+			name: "notice with specific date check",
+			args: args{
+				notice: &model.ProductNotice{
+					Conditions: model.Conditions{
+						DisplayDate: model.NewString(fmt.Sprintf("= %sT00:00:00Z", time.Now().Format("2006-01-02"))),
+					},
+				},
+			},
+			wantErr: false,
+			wantOk:  true,
+		},
 		{
 			name: "notice with date check that doesn't match",
 			args: args{