Add survey config and framework to show feature surveys to skaffold u…

…sers. (#6185)

* add survey config and -id flag to survey command

* add tests

* fix test names

* revert not required

* code review comments

cmd/skaffold/app/cmd/survey.go

@@ -25,13 +25,18 @@ import (
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/survey"
 )
 
+var surveyID string
+
 func NewCmdSurvey() *cobra.Command {
 	return NewCmd("survey").
 		WithDescription("Opens a web browser to fill out the Skaffold survey").
+		WithFlags([]*Flag{
+			{Value: &surveyID, Name: "id", DefValue: survey.HatsID, Usage: "Survey ID for survey command to open."},
+		}).
 		NoArgs(showSurvey)
 }
 
-func showSurvey(context context.Context, out io.Writer) error {
+func showSurvey(ctx context.Context, out io.Writer) error {
 	s := survey.New(opts.GlobalConfig)
-	return s.OpenSurveyForm(context, out)
+	return s.OpenSurveyForm(ctx, out, surveyID)
 }

docs/content/en/docs/references/cli/_index.md

@@ -1125,13 +1125,19 @@ Opens a web browser to fill out the Skaffold survey
 ```
 
 
+Options:
+      --id='hats': Survey ID for survey command to open.
+
 Usage:
   skaffold survey [options]
 
 Use "skaffold options" for a list of global command-line options (applies to all commands).
 
 
 ```
+Env vars:
+
+* `SKAFFOLD_ID` (same as `--id`)
 
 ### skaffold test
 

pkg/skaffold/constants/constants.go

@@ -73,6 +73,9 @@ const (
 	GithubIssueLink = "https://github.com/GoogleContainerTools/skaffold/issues/new"
 
 	Windows = "windows"
+
+	// HaTS is the HaTS Survey ID
+	HaTS = "hats"
 )
 
 type Phase string

pkg/skaffold/survey/config.go

@@ -0,0 +1,113 @@
+/*
+Copyright 2021 The Skaffold Authors
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package survey
+
+import (
+	"fmt"
+	"time"
+
+	sConfig "github.com/GoogleContainerTools/skaffold/pkg/skaffold/config"
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/util"
+)
+
+const (
+	HatsID  = "hats"
+	hatsURL = "https://forms.gle/BMTbGQXLWSdn7vEs6"
+)
+
+var (
+	hats = config{
+		id:         HatsID,
+		promptText: "Help improve Skaffold with our 2-minute anonymous survey",
+		isRelevantFn: func([]util.VersionedConfig, sConfig.RunMode) bool {
+			return true
+		},
+		URL: hatsURL,
+	}
+	// surveys contains all the skaffold survey information
+	surveys = []config{hats}
+)
+
+// config defines a survey.
+type config struct {
+	id string
+	// promptText is shown to the user and should be formatted so each line should fit in < 80 characters.
+	// For example: `As a Helm user, we are requesting your feedback on a proposed change to Skaffold's integration with Helm.`
+	promptText string
+	// startsAt mentions the date after the users survey should be prompted. This will ensure, Skaffold team can finalize the survey
+	// even after release date.
+	startsAt time.Time
+	// expiresAt places a time limit of the user survey. As users are only prompted every two weeks
+	// by design, this time limit should be at least 4 weeks after the upcoming release date to account
+	// for release propagation lag to Cloud SDK and Cloud Shell.
+	expiresAt    time.Time
+	isRelevantFn func([]util.VersionedConfig, sConfig.RunMode) bool
+	URL          string
+}
+
+func (s config) isActive() bool {
+	return s.expiresAt.IsZero() || s.expiresAt.After(time.Now())
+}
+
+func (s config) prompt() string {
+	if s.id == hats.id {
+		return fmt.Sprintf(`%s: run 'skaffold survey'
+`, s.promptText)
+	}
+	return fmt.Sprintf(`%s: run 'skaffold survey -id %s'
+`, s.promptText, s.id)
+}
+
+func (s config) isRelevant(cfgs []util.VersionedConfig, cmd sConfig.RunMode) bool {
+	return s.isRelevantFn(cfgs, cmd)
+}
+
+func (s config) isValid() bool {
+	if s.id == HatsID {
+		return true
+	}
+	today := s.startsAt
+	if today.IsZero() {
+		today = time.Now()
+	}
+	return s.expiresAt.Sub(today) < 60*24*time.Hour
+}
+
+func getSurvey(id string) (config, bool) {
+	for _, s := range surveys {
+		if s.id == id {
+			return s, true
+		}
+	}
+	return config{}, false
+}
+
+func validKeys() []string {
+	keys := make([]string, 0, len(surveys))
+	for _, s := range surveys {
+		keys = append(keys, s.id)
+	}
+	return keys
+}
+
+func init() {
+	for _, s := range surveys {
+		if !s.isValid() {
+			panic(fmt.Errorf("survey %q is valid for more than a 60 days - user surveys must be valid for 60 days or less", s.id))
+		}
+	}
+}

pkg/skaffold/survey/config_test.go

@@ -0,0 +1,233 @@
+/*
+Copyright 2021 The Skaffold Authors
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package survey
+
+import (
+	"testing"
+	"time"
+
+	sConfig "github.com/GoogleContainerTools/skaffold/pkg/skaffold/config"
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/util"
+	"github.com/GoogleContainerTools/skaffold/testutil"
+)
+
+func TestSurveyPrompt(t *testing.T) {
+	tests := []struct {
+		description string
+		s           config
+		expected    string
+	}{
+		{
+			description: "hats survey",
+			s:           hats,
+			expected: `Help improve Skaffold with our 2-minute anonymous survey: run 'skaffold survey'
+`,
+		},
+		{
+			description: "not hats survey",
+			s: config{
+				id:         "foo",
+				promptText: "Looks like you are using foo feature. Help improve Skaffold foo feature and take this survey",
+				expiresAt:  time.Date(2021, time.August, 14, 00, 00, 00, 0, time.UTC),
+			},
+			expected: `Looks like you are using foo feature. Help improve Skaffold foo feature and take this survey: run 'skaffold survey -id foo'
+`,
+		},
+	}
+	for _, test := range tests {
+		testutil.Run(t, test.description, func(t *testutil.T) {
+			t.CheckDeepEqual(test.s.prompt(), test.expected)
+		})
+	}
+}
+
+func TestSurveyActive(t *testing.T) {
+	tests := []struct {
+		description string
+		s           config
+		expected    bool
+	}{
+		{
+			description: "no expiry",
+			s:           hats,
+			expected:    true,
+		},
+		{
+			description: "expiry in past",
+			s: config{
+				id:        "expired",
+				expiresAt: time.Date(2020, 8, 1, 0, 0, 0, 0, time.UTC),
+			},
+		},
+		{
+			description: "expiry in future",
+			s: config{
+				id:        "active",
+				expiresAt: time.Now().AddDate(1, 0, 0),
+			},
+			expected: true,
+		},
+	}
+	for _, test := range tests {
+		testutil.Run(t, test.description, func(t *testutil.T) {
+			t.CheckDeepEqual(test.s.isActive(), test.expected)
+		})
+	}
+}
+
+func TestSurveyRelevant(t *testing.T) {
+	testMock := mockVersionedConfig{version: "test"}
+	prodMock := mockVersionedConfig{version: "prod"}
+
+	tests := []struct {
+		description string
+		s           config
+		cfgs        []util.VersionedConfig
+		expected    bool
+	}{
+		{
+			description: "hats is always relevant",
+			s:           hats,
+			expected:    true,
+		},
+		{
+			description: "relevant based on input configs",
+			s: config{
+				id: "foo",
+				isRelevantFn: func(cfgs []util.VersionedConfig, _ sConfig.RunMode) bool {
+					return len(cfgs) > 1
+				},
+			},
+			cfgs:     []util.VersionedConfig{testMock, prodMock},
+			expected: true,
+		},
+		{
+			description: "not relevant based on config",
+			s: config{
+				id: "foo",
+				isRelevantFn: func(cfgs []util.VersionedConfig, _ sConfig.RunMode) bool {
+					return len(cfgs) > 1
+				},
+			},
+			cfgs: []util.VersionedConfig{testMock},
+		},
+		{
+			description: "contains a config with test version",
+			s: config{
+				id: "version-value-test",
+				isRelevantFn: func(cfgs []util.VersionedConfig, _ sConfig.RunMode) bool {
+					for _, cfg := range cfgs {
+						if m, ok := cfg.(mockVersionedConfig); ok {
+							if m.version == "test" {
+								return true
+							}
+						}
+					}
+					return false
+				},
+			},
+			cfgs:     []util.VersionedConfig{prodMock, testMock},
+			expected: true,
+		},
+		{
+			description: "does not contains a config with test version",
+			s: config{
+				id: "version-value-test",
+				isRelevantFn: func(cfgs []util.VersionedConfig, _ sConfig.RunMode) bool {
+					for _, cfg := range cfgs {
+						if m, ok := cfg.(mockVersionedConfig); ok {
+							if m.version == "test" {
+								return true
+							}
+						}
+					}
+					return false
+				},
+			},
+			cfgs: []util.VersionedConfig{prodMock},
+		},
+	}
+	for _, test := range tests {
+		testutil.Run(t, test.description, func(t *testutil.T) {
+			t.CheckDeepEqual(test.s.isRelevant(test.cfgs, "dev"), test.expected)
+		})
+	}
+}
+
+func TestIsValid(t *testing.T) {
+	tests := []struct {
+		description string
+		s           config
+		expected    bool
+	}{
+		{
+			description: "only hats",
+			s:           hats,
+			expected:    true,
+		},
+		{
+			description: "4 weeks valid survey with start date",
+			s: config{
+				id:        "invalid",
+				startsAt:  time.Now().AddDate(0, 1, 0),
+				expiresAt: time.Now().AddDate(0, 2, 0),
+			},
+			expected: true,
+		},
+		{
+			description: "4 weeks valid survey without start date",
+			s: config{
+				id:        "valid",
+				expiresAt: time.Now().AddDate(0, 1, 0),
+			},
+			expected: true,
+		},
+		{
+			description: "90 days invalid survey without start date",
+			s: config{
+				id:        "invalid",
+				expiresAt: time.Now().AddDate(0, 0, 90),
+			},
+		},
+		{
+			description: "90 days invalid survey with start date",
+			s: config{
+				id:        "invalid",
+				startsAt:  time.Now().AddDate(0, 1, 0),
+				expiresAt: time.Now().AddDate(0, 1, 90),
+			},
+		},
+	}
+	for _, test := range tests {
+		testutil.Run(t, test.description, func(t *testutil.T) {
+			t.CheckDeepEqual(test.s.isValid(), test.expected)
+		})
+	}
+}
+
+// mockVersionedConfig implements util.VersionedConfig.
+type mockVersionedConfig struct {
+	version string
+}
+
+func (m mockVersionedConfig) GetVersion() string {
+	return m.version
+}
+
+func (m mockVersionedConfig) Upgrade() (util.VersionedConfig, error) {
+	return m, nil
+}