Add support for Profiles (#1903)

* Added test case for Profiles

* changed config profiles to be a map, added an accessor method for getting a certain configuration profile

* Added test cases for profile check

* added profile feature

* Fix generated files

* Fix go.mod deps

* Added support for labels and added test cases

---------

Co-authored-by: Roland Huß <rhuss@redhat.com>
Co-authored-by: David Simansky <dsimansk@redhat.com>

docs/cmd/kn_service_apply.md

@@ -60,6 +60,7 @@ kn service apply s0 --filename my-svc.yml
       --probe-liveness-opts string        Add common options to liveness probe. Common opts (comma separated, case insensitive): InitialDelaySeconds=<int_value>, FailureThreshold=<int_value>, SuccessThreshold=<int_value>, PeriodSeconds=<int_value>, TimeoutSeconds=<int_value>
       --probe-readiness string            Add readiness probe to Service deployment. Supported probe types are HTTGet, Exec and TCPSocket. Format: [http,https]:host:port:path, exec:cmd[,cmd,...], tcp:host:port.
       --probe-readiness-opts string       Add common options to readiness probe. Common opts (comma separated, case insensitive): InitialDelaySeconds=<int_value>, FailureThreshold=<int_value>, SuccessThreshold=<int_value>, PeriodSeconds=<int_value>, TimeoutSeconds=<int_value>
+      --profile string                    The profile name must be defined in config.yaml or part of the built-in profile, e.g. Istio. Related annotations and labels will be added to the service.To unset, specify the profile name followed by a "-" (e.g., name-).
       --pull-policy string                Image pull policy. Valid values (case insensitive): Always | Never | IfNotPresent
       --pull-secret string                Image pull secret to set. An empty argument ("") clears the pull secret. The referenced secret must exist in the service's namespace.
       --request strings                   The resource requirement requests for this Service. For example, 'cpu=100m,memory=256Mi'. You can use this flag multiple times. To unset a resource request, append "-" to the resource name, e.g. '--request cpu-'.

docs/cmd/kn_service_create.md

@@ -49,6 +49,9 @@ kn service create NAME --image IMAGE
   kn service create gitopstest -n test-ns --image knativesamples/helloworld --target=/user/knfiles
   kn service create gitopstest --image knativesamples/helloworld --target=/user/knfiles/test.yaml
   kn service create gitopstest --image knativesamples/helloworld --target=/user/knfiles/test.json
+
+  # Create a service with profile
+  kn service create profiletest --image knativesamples/helloworld --profile istio
 ```
 
 ### Options
@@ -85,6 +88,7 @@ kn service create NAME --image IMAGE
       --probe-liveness-opts string        Add common options to liveness probe. Common opts (comma separated, case insensitive): InitialDelaySeconds=<int_value>, FailureThreshold=<int_value>, SuccessThreshold=<int_value>, PeriodSeconds=<int_value>, TimeoutSeconds=<int_value>
       --probe-readiness string            Add readiness probe to Service deployment. Supported probe types are HTTGet, Exec and TCPSocket. Format: [http,https]:host:port:path, exec:cmd[,cmd,...], tcp:host:port.
       --probe-readiness-opts string       Add common options to readiness probe. Common opts (comma separated, case insensitive): InitialDelaySeconds=<int_value>, FailureThreshold=<int_value>, SuccessThreshold=<int_value>, PeriodSeconds=<int_value>, TimeoutSeconds=<int_value>
+      --profile string                    The profile name must be defined in config.yaml or part of the built-in profile, e.g. Istio. Related annotations and labels will be added to the service.To unset, specify the profile name followed by a "-" (e.g., name-).
       --pull-policy string                Image pull policy. Valid values (case insensitive): Always | Never | IfNotPresent
       --pull-secret string                Image pull secret to set. An empty argument ("") clears the pull secret. The referenced secret must exist in the service's namespace.
       --request strings                   The resource requirement requests for this Service. For example, 'cpu=100m,memory=256Mi'. You can use this flag multiple times. To unset a resource request, append "-" to the resource name, e.g. '--request cpu-'.

docs/cmd/kn_service_update.md

@@ -72,6 +72,7 @@ kn service update NAME
       --probe-liveness-opts string        Add common options to liveness probe. Common opts (comma separated, case insensitive): InitialDelaySeconds=<int_value>, FailureThreshold=<int_value>, SuccessThreshold=<int_value>, PeriodSeconds=<int_value>, TimeoutSeconds=<int_value>
       --probe-readiness string            Add readiness probe to Service deployment. Supported probe types are HTTGet, Exec and TCPSocket. Format: [http,https]:host:port:path, exec:cmd[,cmd,...], tcp:host:port.
       --probe-readiness-opts string       Add common options to readiness probe. Common opts (comma separated, case insensitive): InitialDelaySeconds=<int_value>, FailureThreshold=<int_value>, SuccessThreshold=<int_value>, PeriodSeconds=<int_value>, TimeoutSeconds=<int_value>
+      --profile string                    The profile name must be defined in config.yaml or part of the built-in profile, e.g. Istio. Related annotations and labels will be added to the service.To unset, specify the profile name followed by a "-" (e.g., name-).
       --pull-policy string                Image pull policy. Valid values (case insensitive): Always | Never | IfNotPresent
       --pull-secret string                Image pull secret to set. An empty argument ("") clears the pull secret. The referenced secret must exist in the service's namespace.
       --request strings                   The resource requirement requests for this Service. For example, 'cpu=100m,memory=256Mi'. You can use this flag multiple times. To unset a resource request, append "-" to the resource name, e.g. '--request cpu-'.

pkg/kn/commands/service/configuration_edit_flags.go

@@ -24,6 +24,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"knative.dev/serving/pkg/apis/config"
 
+	knconfig "knative.dev/client/pkg/kn/config"
 	knflags "knative.dev/client/pkg/kn/flags"
 	servinglib "knative.dev/client/pkg/serving"
 	"knative.dev/client/pkg/util"
@@ -57,6 +58,7 @@ type ConfigurationEditFlags struct {
 	ClusterLocal        bool
 	ScaleInit           int
 	TimeoutSeconds      int64
+	Profile             string
 
 	// Preferences about how to do the action.
 	LockToDigest         bool
@@ -187,6 +189,11 @@ func (p *ConfigurationEditFlags) addSharedFlags(command *cobra.Command) {
 		"Duration in seconds that the request routing layer will wait for a request delivered to a "+""+
 			"container to begin replying")
 	p.markFlagMakesRevision("timeout")
+
+	command.Flags().StringVar(&p.Profile, "profile", "",
+		"The profile name must be defined in config.yaml or part of the built-in profile, e.g. Istio. Related annotations and labels will be added to the service."+
+			"To unset, specify the profile name followed by a \"-\" (e.g., name-).")
+	p.markFlagMakesRevision("profile")
 }
 
 // AddUpdateFlags adds the flags specific to update.
@@ -479,6 +486,63 @@ func (p *ConfigurationEditFlags) Apply(
 		service.Spec.Template.Spec.TimeoutSeconds = &p.TimeoutSeconds
 	}
 
+	if cmd.Flags().Changed("profile") {
+		profileName := ""
+		deleteProfile := false
+		if strings.HasSuffix(p.Profile, "-") {
+			profileName = p.Profile[:len(p.Profile)-1]
+			deleteProfile = true
+		} else {
+			profileName = p.Profile
+			deleteProfile = false
+		}
+
+		if len(knconfig.GlobalConfig.Profile(profileName).Annotations) > 0 || len(knconfig.GlobalConfig.Profile(profileName).Labels) > 0 {
+			annotations := knconfig.GlobalConfig.Profile(profileName).Annotations
+			labels := knconfig.GlobalConfig.Profile(profileName).Labels
+
+			profileAnnotations := make(util.StringMap)
+			for _, value := range annotations {
+				profileAnnotations[value.Name] = value.Value
+			}
+
+			profileLabels := make(util.StringMap)
+			for _, value := range labels {
+				profileLabels[value.Name] = value.Value
+			}
+
+			if deleteProfile {
+				var annotationsToRemove []string
+				for _, value := range annotations {
+					annotationsToRemove = append(annotationsToRemove, value.Name)
+				}
+
+				var labelsToRemove []string
+				for _, value := range labels {
+					labelsToRemove = append(labelsToRemove, value.Name)
+				}
+
+				if err = servinglib.UpdateRevisionTemplateAnnotations(template, map[string]string{}, annotationsToRemove); err != nil {
+					return err
+				}
+				updatedLabels := servinglib.UpdateLabels(service.ObjectMeta.Labels, map[string]string{}, labelsToRemove)
+				service.ObjectMeta.Labels = updatedLabels // In case service.ObjectMeta.Labels was nil
+			} else {
+				if err = servinglib.UpdateRevisionTemplateAnnotations(template, profileAnnotations, []string{}); err != nil {
+					return err
+				}
+				updatedLabels := servinglib.UpdateLabels(service.ObjectMeta.Labels, profileLabels, []string{})
+				service.ObjectMeta.Labels = updatedLabels // In case service.ObjectMeta.Labels was nil
+			}
+
+			if err != nil {
+				return err
+			}
+		} else {
+			return fmt.Errorf("profile %s doesn't exist", profileName)
+		}
+	}
+
 	return nil
 }
 

pkg/kn/commands/service/configuration_edit_flags_test.go

@@ -15,10 +15,15 @@
 package service
 
 import (
+	"os"
+	"path/filepath"
 	"testing"
 
+	"github.com/spf13/viper"
 	"gotest.tools/v3/assert"
+
 	"knative.dev/client/pkg/kn/commands"
+	"knative.dev/client/pkg/kn/config"
 	"knative.dev/client/pkg/util"
 	"knative.dev/serving/pkg/apis/autoscaling"
 )
@@ -75,3 +80,138 @@ func TestScaleActivation(t *testing.T) {
 	assert.NilError(t, err)
 	assert.Equal(t, svc.Spec.Template.Annotations[autoscaling.ActivationScaleKey], "2")
 }
+
+func TestApplyDefaultProfileFlag(t *testing.T) {
+	var editFlags ConfigurationEditFlags
+	knParams := &commands.KnParams{}
+	cmd, _, _ := commands.CreateTestKnCommand(NewServiceCreateCommand(knParams), knParams)
+
+	editFlags.AddCreateFlags(cmd)
+
+	err := config.BootstrapConfig()
+	assert.NilError(t, err)
+
+	svc := createTestService("test-svc", []string{"test-svc-00001"}, goodConditions())
+	cmd.SetArgs([]string{"--profile", "istio"})
+	cmd.Execute()
+	editFlags.Apply(&svc, nil, cmd)
+	assert.Equal(t, svc.Spec.Template.Annotations["sidecar.istio.io/inject"], "true")
+	assert.Equal(t, svc.Spec.Template.Annotations["sidecar.istio.io/rewriteAppHTTPProbers"], "true")
+	assert.Equal(t, svc.Spec.Template.Annotations["serving.knative.openshift.io/enablePassthrough"], "true")
+}
+
+func TestApplyProfileFlag(t *testing.T) {
+	var editFlags ConfigurationEditFlags
+	knParams := &commands.KnParams{}
+	cmd, _, _ := commands.CreateTestKnCommand(NewServiceCreateCommand(knParams), knParams)
+	configYaml := `
+profiles:
+  testprofile:
+      labels:
+        - name: environment
+          value: "test"
+      annotations:
+        - name: sidecar.testprofile.io/inject
+          value: "true"
+        - name: sidecar.testprofile.io/rewriteAppHTTPProbers
+          value: "true"
+`
+	_, cleanup := setupConfig(t, configYaml)
+	defer cleanup()
+
+	editFlags.AddCreateFlags(cmd)
+
+	err := config.BootstrapConfig()
+	assert.NilError(t, err)
+
+	svc := createTestService("test-svc", []string{"test-svc-00001"}, goodConditions())
+	cmd.SetArgs([]string{"--profile", "testprofile"})
+	cmd.Execute()
+	editFlags.Apply(&svc, nil, cmd)
+	assert.Equal(t, svc.Spec.Template.Annotations["sidecar.testprofile.io/inject"], "true")
+	assert.Equal(t, svc.Spec.Template.Annotations["sidecar.testprofile.io/rewriteAppHTTPProbers"], "true")
+	assert.Equal(t, svc.ObjectMeta.Labels["environment"], "test")
+}
+
+func TestDeleteProfileFlag(t *testing.T) {
+	var editFlags ConfigurationEditFlags
+	knParams := &commands.KnParams{}
+	cmd, _, _ := commands.CreateTestKnCommand(NewServiceCreateCommand(knParams), knParams)
+	configYaml := `
+profiles:
+  testprofile:
+    labels:
+      - name: environment
+        value: "test"
+    annotations:
+      - name: sidecar.testprofile.io/inject
+        value: "true"
+      - name: sidecar.testprofile.io/rewriteAppHTTPProbers
+        value: "true"
+`
+	_, cleanup := setupConfig(t, configYaml)
+	defer cleanup()
+
+	editFlags.AddCreateFlags(cmd)
+
+	err := config.BootstrapConfig()
+	assert.NilError(t, err)
+
+	svc := createTestService("test-svc", []string{"test-svc-00001"}, goodConditions())
+	cmd.SetArgs([]string{"--profile", "testprofile"})
+	cmd.Execute()
+	editFlags.Apply(&svc, nil, cmd)
+	assert.Equal(t, svc.Spec.Template.Annotations["sidecar.testprofile.io/inject"], "true")
+	assert.Equal(t, svc.Spec.Template.Annotations["sidecar.testprofile.io/rewriteAppHTTPProbers"], "true")
+	assert.Equal(t, svc.ObjectMeta.Labels["environment"], "test")
+
+	cmd.SetArgs([]string{"--profile", "testprofile-"})
+	cmd.Execute()
+	editFlags.Apply(&svc, nil, cmd)
+
+	assert.Equal(t, svc.Spec.Template.Annotations["sidecar.istio.io/inject"], "")
+	assert.Equal(t, len(svc.Spec.Template.Annotations), 1)
+
+	assert.Equal(t, svc.ObjectMeta.Labels["environment"], "")
+}
+
+func TestApplyProfileFlagError(t *testing.T) {
+	var editFlags ConfigurationEditFlags
+	knParams := &commands.KnParams{}
+	cmd, _, _ := commands.CreateTestKnCommand(NewServiceCreateCommand(knParams), knParams)
+	editFlags.AddCreateFlags(cmd)
+	err := config.BootstrapConfig()
+	assert.NilError(t, err)
+
+	svc := createTestService("test-svc", []string{"test-svc-00001"}, goodConditions())
+	cmd.SetArgs([]string{"--profile", "invalidprofile"})
+	cmd.Execute()
+	err = editFlags.Apply(&svc, nil, cmd)
+	assert.Assert(t, util.ContainsAll(err.Error(), "profile", "invalidprofile"))
+}
+
+func setupConfig(t *testing.T, configContent string) (string, func()) {
+	tmpDir := t.TempDir()
+
+	// Avoid to be fooled by the things in the the real homedir
+	oldHome := os.Getenv("HOME")
+	os.Setenv("HOME", tmpDir)
+
+	// Save old args
+	backupArgs := os.Args
+
+	// WriteCache out a temporary configContent file
+	var cfgFile string
+	if configContent != "" {
+		cfgFile = filepath.Join(tmpDir, "config.yaml")
+		os.Args = []string{"kn", "--config", cfgFile}
+		err := os.WriteFile(cfgFile, []byte(configContent), 0644)
+		assert.NilError(t, err)
+	}
+	return cfgFile, func() {
+		// Cleanup everything
+		os.Setenv("HOME", oldHome)
+		os.Args = backupArgs
+		viper.Reset()
+	}
+}

pkg/kn/commands/service/create.go

@@ -80,7 +80,10 @@ var create_example = `
   # Create the service in offline mode instead of kubernetes cluster (Beta)
   kn service create gitopstest -n test-ns --image knativesamples/helloworld --target=/user/knfiles
   kn service create gitopstest --image knativesamples/helloworld --target=/user/knfiles/test.yaml
-  kn service create gitopstest --image knativesamples/helloworld --target=/user/knfiles/test.json`
+  kn service create gitopstest --image knativesamples/helloworld --target=/user/knfiles/test.json
+
+  # Create a service with profile
+  kn service create profiletest --image knativesamples/helloworld --profile istio`
 
 func NewServiceCreateCommand(p *commands.KnParams) *cobra.Command {
 	var editFlags ConfigurationEditFlags

pkg/kn/config/config.go

@@ -64,6 +64,9 @@ type config struct {
 
 	// channelTypeMappings is a list of channel type mapping
 	channelTypeMappings []ChannelTypeMapping
+
+	// profiles is a map of profiles from the config file and built-in profiles
+	profiles map[string]Profile
 }
 
 func (c *config) ContextSharing() bool {
@@ -100,6 +103,10 @@ func (c *config) SinkMappings() []SinkMapping {
 	return c.sinkMappings
 }
 
+func (c *config) Profile(profile string) Profile {
+	return c.profiles[profile]
+}
+
 func (c *config) ChannelTypeMappings() []ChannelTypeMapping {
 	return c.channelTypeMappings
 }
@@ -167,6 +174,12 @@ func BootstrapConfig() error {
 		return err
 	}
 
+	// Deserialize profiles if configured
+	err = parseProfiles()
+	if err != nil {
+		return err
+	}
+
 	// Deserialize channel type mappings if configured
 	err = parseChannelTypeMappings()
 	return err
@@ -267,6 +280,49 @@ func parseSinkMappings() error {
 	return nil
 }
 
+// parse profiles and store them in the global configuration
+func parseProfiles() error {
+	if viper.IsSet(profiles) {
+		err := viper.UnmarshalKey(profiles, &globalConfig.profiles)
+		if err != nil {
+			return fmt.Errorf("error while parsing profiles in configuration file %s: %w",
+				viper.ConfigFileUsed(), err)
+		}
+	}
+	globalConfig.profiles = mergeProfilesWithBuiltInProfiles(globalConfig.profiles)
+
+	return nil
+}
+
+// defaultProfiles returns the built-in profiles
+func builtInProfiles() map[string]Profile {
+	return map[string]Profile{
+		istio: {
+			Annotations: []NamedValue{
+				{Name: "sidecar.istio.io/inject", Value: "true"},
+				{Name: "sidecar.istio.io/rewriteAppHTTPProbers", Value: "true"},
+				{Name: "serving.knative.openshift.io/enablePassthrough", Value: "true"},
+			},
+		},
+	}
+}
+
+// mergeProfilesWithDefaultProfiles merges the given profiles with the built-in profiles
+func mergeProfilesWithBuiltInProfiles(profiles map[string]Profile) map[string]Profile {
+	builtInProfiles := builtInProfiles()
+	mergedProfiles := make(map[string]Profile, len(builtInProfiles)+len(profiles))
+
+	for key, value := range builtInProfiles {
+		mergedProfiles[key] = value
+	}
+
+	for key, value := range profiles {
+		mergedProfiles[key] = value
+	}
+
+	return mergedProfiles
+}
+
 // parse channel type mappings and store them in the global configuration
 func parseChannelTypeMappings() error {
 	if viper.IsSet(keyChannelTypeMappings) {

pkg/kn/config/config_test.go

@@ -24,10 +24,27 @@ import (
 	"gotest.tools/v3/assert"
 )
 
+// TestDefaultProfile tests that the default profile is correctly loaded
+func TestDefaultProfile(t *testing.T) {
+	profile := builtInProfiles()
+	assert.Equal(t, len(profile[istio].Labels), 0)
+	assert.Equal(t, len(profile[istio].Annotations), 3)
+}
+
 func TestBootstrapConfig(t *testing.T) {
 	configYaml := `
 plugins:
   directory: /tmp
+profiles:
+  knative:
+    labels:
+      - name: environment
+        value: "knative"
+    annotations:
+      - name: sidecar.knative.io/inject
+        value: "true"
+      - name: sidecar.knative.io/rewriteAppHTTPProbers
+        value: "true"
 eventing:
   sink-mappings:
   - prefix: service
@@ -51,6 +68,38 @@ eventing:
 	assert.Equal(t, GlobalConfig.PluginsDir(), "/tmp")
 	assert.Equal(t, GlobalConfig.LookupPluginsInPath(), true)
 	assert.Equal(t, len(GlobalConfig.SinkMappings()), 1)
+	assert.Equal(t, len(GlobalConfig.Profile("istio").Annotations), 3)
+	assert.DeepEqual(t, GlobalConfig.Profile("istio").Annotations, []NamedValue{
+		{
+			Name:  "sidecar.istio.io/inject",
+			Value: "true",
+		},
+		{
+			Name:  "sidecar.istio.io/rewriteAppHTTPProbers",
+			Value: "true",
+		},
+		{
+			Name:  "serving.knative.openshift.io/enablePassthrough",
+			Value: "true",
+		},
+	})
+	assert.DeepEqual(t, GlobalConfig.Profile("knative").Annotations, []NamedValue{
+		{
+			Name:  "sidecar.knative.io/inject",
+			Value: "true",
+		},
+		{
+			Name:  "sidecar.knative.io/rewriteAppHTTPProbers",
+			Value: "true",
+		},
+	})
+	assert.Equal(t, len(GlobalConfig.Profile("knative").Labels), 1)
+	assert.DeepEqual(t, GlobalConfig.Profile("knative").Labels, []NamedValue{
+		{
+			Name:  "environment",
+			Value: "knative",
+		},
+	})
 	assert.DeepEqual(t, (GlobalConfig.SinkMappings())[0], SinkMapping{
 		Prefix:   "service",
 		Resource: "services",

pkg/kn/config/testing.go

@@ -24,6 +24,7 @@ type TestConfig struct {
 	TestLookupPluginsInPath bool
 	TestSinkMappings        []SinkMapping
 	TestChannelTypeMappings []ChannelTypeMapping
+	TestProfiles            map[string]Profile
 }
 
 // Ensure that TestConfig implements the configuration interface
@@ -35,3 +36,4 @@ func (t TestConfig) ConfigFile() string                        { return t.TestCo
 func (t TestConfig) LookupPluginsInPath() bool                 { return t.TestLookupPluginsInPath }
 func (t TestConfig) SinkMappings() []SinkMapping               { return t.TestSinkMappings }
 func (t TestConfig) ChannelTypeMappings() []ChannelTypeMapping { return t.TestChannelTypeMappings }
+func (t TestConfig) Profile(profile string) Profile            { return t.TestProfiles[profile] }

pkg/kn/config/types.go

@@ -37,6 +37,9 @@ type Config interface {
 
 	// ChannelTypeMappings returns additional mappings for channel type aliases
 	ChannelTypeMappings() []ChannelTypeMapping
+
+	// Profile returns a configured profile with this name or nil of no such profile is configured
+	Profile(profile string) Profile
 }
 
 // SinkMappings is the struct of sink prefix config in kn config
@@ -71,12 +74,25 @@ type ChannelTypeMapping struct {
 	Version string
 }
 
+// NamedValue is the struct of name and values in the Profile struct
+type NamedValue struct {
+	Name  string `yaml:"name"`
+	Value string `yaml:"value"`
+}
+
+// Profile is the struct of profile config in kn config
+type Profile struct {
+	Annotations []NamedValue `yaml:"annotations"`
+	Labels      []NamedValue `yaml:"labels"`
+}
+
 // config Keys for looking up in viper
 const (
 	keyFeaturesContextSharing = "features.context-sharing"
 	keyPluginsDirectory       = "plugins.directory"
 	keySinkMappings           = "eventing.sink-mappings"
 	keyChannelTypeMappings    = "eventing.channel-type-mappings"
+	profiles                  = "profiles"
 )
 
 // legacy config keys, deprecated
@@ -90,3 +106,8 @@ const (
 const (
 	flagPluginsDir = "plugins-dir"
 )
+
+// default profiles
+const (
+	istio = "istio"
+)