Add default SecurityContext to every new ksvc (#1821)

* Add default SecurityContext to every new ksvc

* Add basic seccon config flag

* Fix export e2e tests

* Fix unit test

docs/cmd/kn_container_add.md

@@ -47,6 +47,7 @@ kn container add 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-'.
+      --security-context string       Security Context definition to be added the service. Accepted values: strict | none. (default "strict")
       --service-account string        Service account name to set. An empty argument ("") clears the service account. The referenced service account must exist in the service's namespace.
       --user int                      The user ID to run the container (e.g., 1001).
 ```

docs/cmd/kn_service_apply.md

@@ -73,6 +73,7 @@ kn service apply s0 --filename my-svc.yml
       --scale-target int                  Recommendation for what metric value the PodAutoscaler should attempt to maintain. Use with --scale-metric flag to configure the metric name for which the target value should be maintained. Default metric name is concurrency. The flag defaults to --concurrency-limit when given.
       --scale-utilization int             Percentage of concurrent requests utilization before scaling up. (default 70)
       --scale-window string               Duration to look back for making auto-scaling decisions. The service is scaled to zero if no request was received in during that time. (eg: 10s)
+      --security-context string           Security Context definition to be added the service. Accepted values: strict | none. (default "strict")
       --service-account string            Service account name to set. An empty argument ("") clears the service account. The referenced service account must exist in the service's namespace.
       --timeout int                       Duration in seconds that the request routing layer will wait for a request delivered to a container to begin replying (default 300)
       --user int                          The user ID to run the container (e.g., 1001).

docs/cmd/kn_service_create.md

@@ -98,6 +98,7 @@ kn service create NAME --image IMAGE
       --scale-target int                  Recommendation for what metric value the PodAutoscaler should attempt to maintain. Use with --scale-metric flag to configure the metric name for which the target value should be maintained. Default metric name is concurrency. The flag defaults to --concurrency-limit when given.
       --scale-utilization int             Percentage of concurrent requests utilization before scaling up. (default 70)
       --scale-window string               Duration to look back for making auto-scaling decisions. The service is scaled to zero if no request was received in during that time. (eg: 10s)
+      --security-context string           Security Context definition to be added the service. Accepted values: strict | none. (default "strict")
       --service-account string            Service account name to set. An empty argument ("") clears the service account. The referenced service account must exist in the service's namespace.
       --tag strings                       Set tag (format: --tag revisionRef=tagName) where revisionRef can be a revision or '@latest' string representing latest ready revision. This flag can be specified multiple times.
       --target string                     Work on local directory instead of a remote cluster (experimental)

docs/cmd/kn_service_update.md

@@ -85,6 +85,7 @@ kn service update NAME
       --scale-target int                  Recommendation for what metric value the PodAutoscaler should attempt to maintain. Use with --scale-metric flag to configure the metric name for which the target value should be maintained. Default metric name is concurrency. The flag defaults to --concurrency-limit when given.
       --scale-utilization int             Percentage of concurrent requests utilization before scaling up. (default 70)
       --scale-window string               Duration to look back for making auto-scaling decisions. The service is scaled to zero if no request was received in during that time. (eg: 10s)
+      --security-context string           Security Context definition to be added the service. Accepted values: strict | none. (default "strict")
       --service-account string            Service account name to set. An empty argument ("") clears the service account. The referenced service account must exist in the service's namespace.
       --tag strings                       Set tag (format: --tag revisionRef=tagName) where revisionRef can be a revision or '@latest' string representing latest ready revision. This flag can be specified multiple times.
       --target string                     Work on local directory instead of a remote cluster (experimental)

docs/cmd/kn_source_container_create.md

@@ -37,6 +37,7 @@ kn source container create NAME --image IMAGE --sink SINK
       --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-'.
+      --security-context string       Security Context definition to be added the service. Accepted values: strict | none. (default "strict")
       --service-account string        Service account name to set. An empty argument ("") clears the service account. The referenced service account must exist in the service's namespace.
   -s, --sink string                   Addressable sink for events. You can specify a broker, channel, Knative service or URI. Examples: '--sink broker:nest' for a broker 'nest', '--sink channel:pipe' for a channel 'pipe', '--sink ksvc:mysvc:mynamespace' for a Knative service 'mysvc' in another namespace 'mynamespace', '--sink https://event.receiver.uri' for an URI with an 'http://' or 'https://' schema, '--sink ksvc:receiver' or simply '--sink receiver' for a Knative service 'receiver' in the current namespace. If a prefix is not provided, it is considered as a Knative service in the current namespace. If referring to a Knative service in another namespace, 'ksvc:name:namespace' combination must be provided explicitly.
       --user int                      The user ID to run the container (e.g., 1001).

docs/cmd/kn_source_container_update.md

@@ -37,6 +37,7 @@ kn source container update NAME --image IMAGE
       --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-'.
+      --security-context string       Security Context definition to be added the service. Accepted values: strict | none. (default "strict")
       --service-account string        Service account name to set. An empty argument ("") clears the service account. The referenced service account must exist in the service's namespace.
   -s, --sink string                   Addressable sink for events. You can specify a broker, channel, Knative service or URI. Examples: '--sink broker:nest' for a broker 'nest', '--sink channel:pipe' for a channel 'pipe', '--sink ksvc:mysvc:mynamespace' for a Knative service 'mysvc' in another namespace 'mynamespace', '--sink https://event.receiver.uri' for an URI with an 'http://' or 'https://' schema, '--sink ksvc:receiver' or simply '--sink receiver' for a Knative service 'receiver' in the current namespace. If a prefix is not provided, it is considered as a Knative service in the current namespace. If referring to a Knative service in another namespace, 'ksvc:name:namespace' combination must be provided explicitly.
       --user int                      The user ID to run the container (e.g., 1001).

go.mod

@@ -28,6 +28,8 @@ require (
 	sigs.k8s.io/yaml v1.3.0
 )
 
+require k8s.io/utils v0.0.0-20221108210102-8e77b1f39fe2
+
 require (
 	contrib.go.opencensus.io/exporter/ocagent v0.7.1-0.20200907061046-05415f1de66d // indirect
 	contrib.go.opencensus.io/exporter/prometheus v0.4.2 // indirect
@@ -122,7 +124,6 @@ require (
 	k8s.io/gengo v0.0.0-20221011193443-fad74ee6edd9 // indirect
 	k8s.io/klog/v2 v2.80.2-0.20221028030830-9ae4992afb54 // indirect
 	k8s.io/kube-openapi v0.0.0-20221012153701-172d655c2280 // indirect
-	k8s.io/utils v0.0.0-20221108210102-8e77b1f39fe2 // indirect
 	sigs.k8s.io/json v0.0.0-20220713155537-f223a00ba0e2 // indirect
 	sigs.k8s.io/kustomize/api v0.12.1 // indirect
 	sigs.k8s.io/kustomize/kyaml v0.13.9 // indirect

lib/test/service.go

@@ -259,6 +259,7 @@ func BuildServiceWithOptions(name string, so ...servingtest.ServiceOption) *serv
 		APIVersion: "serving.knative.dev/v1",
 	}
 	svc.Spec.Template.Spec.Containers[0].Resources = corev1.ResourceRequirements{}
+	svc.Spec.Template.Spec.Containers[0].SecurityContext = &corev1.SecurityContext{}
 	return svc
 }
 
@@ -301,6 +302,7 @@ func BuildRevision(name string, options ...servingtest.RevisionOption) *servingv
 	rev.ObjectMeta.UID = ""
 	rev.ObjectMeta.Generation = int64(0)
 	rev.Spec.PodSpec.Containers[0].Resources = corev1.ResourceRequirements{}
+	rev.Spec.PodSpec.Containers[0].SecurityContext = &corev1.SecurityContext{}
 	return rev
 }
 

pkg/kn/commands/service/create_mock_test.go

@@ -18,6 +18,8 @@ import (
 	"testing"
 	"time"
 
+	"knative.dev/client/pkg/kn/flags"
+
 	"knative.dev/serving/pkg/apis/autoscaling"
 
 	"gotest.tools/v3/assert"
@@ -477,6 +479,7 @@ func getService(name string) *servingv1.Service {
 			Limits:   corev1.ResourceList{},
 			Requests: corev1.ResourceList{},
 		},
+		SecurityContext: flags.DefaultStrictSecCon(),
 	}}
 
 	return service

pkg/kn/commands/source/container/container_test.go

@@ -20,6 +20,8 @@ import (
 	"bytes"
 	"strings"
 
+	"knative.dev/client/pkg/kn/flags"
+
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/client-go/tools/clientcmd"
 	v1 "knative.dev/eventing/pkg/apis/sources/v1"
@@ -92,6 +94,7 @@ func createContainerSource(name, image string, sink duckv1.Destination, ceo map[
 					Limits:   corev1.ResourceList{},
 					Requests: corev1.ResourceList{},
 				},
+				SecurityContext: flags.DefaultStrictSecCon(),
 			}}}).
 		Sink(sink).
 		Build()

pkg/kn/flags/podspec.go

@@ -52,6 +52,8 @@ type PodSpecFlags struct {
 	ServiceAccountName string
 	ImagePullSecrets   string
 	User               int64
+
+	SecurityContext string
 }
 
 type ResourceFlags struct {
@@ -234,6 +236,10 @@ func (p *PodSpecFlags) AddFlags(flagset *pflag.FlagSet) []string {
 	flagNames = append(flagNames, "pull-secret")
 	flagset.Int64VarP(&p.User, "user", "", 0, "The user ID to run the container (e.g., 1001).")
 	flagNames = append(flagNames, "user")
+
+	flagset.StringVar(&p.SecurityContext, "security-context", "strict", "Security Context definition to be added the service. Accepted values: strict | none.")
+	flagNames = append(flagNames, "security-context")
+
 	return flagNames
 }
 
@@ -405,5 +411,15 @@ func (p *PodSpecFlags) ResolvePodSpec(podSpec *corev1.PodSpec, flags *pflag.Flag
 		}
 	}
 
+	if flags.Changed("security-context") {
+		if err := UpdateSecurityContext(podSpec, p.SecurityContext); err != nil {
+			return err
+		}
+	} else {
+		if err := UpdateSecurityContext(podSpec, ""); err != nil {
+			return err
+		}
+	}
+
 	return nil
 }

pkg/kn/flags/podspec_helper.go

@@ -20,6 +20,8 @@ import (
 	"strconv"
 	"strings"
 
+	"k8s.io/utils/pointer"
+
 	"k8s.io/apimachinery/pkg/util/intstr"
 
 	"k8s.io/apimachinery/pkg/api/resource"
@@ -378,6 +380,42 @@ func UpdateImagePullPolicy(spec *corev1.PodSpec, imagePullPolicy string) error {
 	return nil
 }
 
+// UpdateSecurityContext update the Security Context
+func UpdateSecurityContext(spec *corev1.PodSpec, securityContext string) error {
+	container := containerOfPodSpec(spec)
+	switch strings.ToLower(securityContext) {
+	case "none":
+		// Blank any Security Context defined
+		container.SecurityContext = &corev1.SecurityContext{}
+	case "strict":
+		// Add or update Security Context to default strict
+		container.SecurityContext = DefaultStrictSecCon()
+	case "":
+		// Add default strict SC flag is not used, hence empty value
+		if container.SecurityContext == nil {
+			container.SecurityContext = DefaultStrictSecCon()
+		}
+		//TODO(dsimansk): add parsing of SC options from the flag value
+	default:
+		return fmt.Errorf("invalid --security-context %s. Valid arguments: strict | none", securityContext)
+	}
+	return nil
+}
+
+// DefaultStrictSecCon helper function to get default strict Security Context
+func DefaultStrictSecCon() *corev1.SecurityContext {
+	return &corev1.SecurityContext{
+		AllowPrivilegeEscalation: pointer.Bool(false),
+		RunAsNonRoot:             pointer.Bool(true),
+		Capabilities: &corev1.Capabilities{
+			Drop: []corev1.Capability{"ALL"},
+		},
+		SeccompProfile: &corev1.SeccompProfile{
+			Type: corev1.SeccompProfileTypeRuntimeDefault,
+		},
+	}
+}
+
 func getPolicy(policy string) v1.PullPolicy {
 	var ret v1.PullPolicy
 	switch strings.ToLower(policy) {

pkg/kn/flags/podspec_helper_test.go

@@ -1487,3 +1487,58 @@ func TestResolveProbeOptions(t *testing.T) {
 		})
 	}
 }
+
+func TestUpdateSecurityContext(t *testing.T) {
+	testCases := []struct {
+		name string
+
+		expected      *corev1.PodSpec
+		expectedError error
+	}{
+		{
+			name: "strict",
+			expected: &corev1.PodSpec{
+				Containers: []corev1.Container{
+					{SecurityContext: DefaultStrictSecCon()}},
+			},
+			expectedError: nil,
+		},
+		{
+			name: "none",
+			expected: &corev1.PodSpec{
+				Containers: []corev1.Container{{
+					SecurityContext: &corev1.SecurityContext{}}},
+			},
+			expectedError: nil,
+		},
+		{
+			name: "",
+			expected: &corev1.PodSpec{
+				Containers: []corev1.Container{
+					{SecurityContext: DefaultStrictSecCon()}},
+			},
+			expectedError: nil,
+		},
+		{
+			name: "unknown",
+			expected: &corev1.PodSpec{
+				Containers: []corev1.Container{
+					{SecurityContext: DefaultStrictSecCon()}},
+			},
+			expectedError: errors.New("invalid --security-context unknown. Valid arguments: strict | none"),
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			actual := &corev1.PodSpec{}
+			err := UpdateSecurityContext(actual, tc.name)
+			if tc.expectedError != nil {
+				assert.Error(t, err, tc.expectedError.Error())
+			} else {
+				assert.NilError(t, err)
+				assert.DeepEqual(t, actual, tc.expected)
+			}
+		})
+	}
+}

pkg/kn/flags/podspec_test.go

@@ -29,7 +29,6 @@ import (
 	"github.com/spf13/cobra"
 	"gotest.tools/v3/assert"
 	corev1 "k8s.io/api/core/v1"
-	v1 "k8s.io/api/core/v1"
 	"knative.dev/client/pkg/util"
 	"knative.dev/pkg/ptr"
 )
@@ -37,14 +36,15 @@ import (
 func TestPodSpecFlags(t *testing.T) {
 	args := []string{"--image", "repo/user/imageID:tag", "--env", "b=c"}
 	wantedPod := &PodSpecFlags{
-		Image:        "repo/user/imageID:tag",
+		Image:           "repo/user/imageID:tag",
-		Env:          []string{"b=c"},
+		Env:             []string{"b=c"},
-		EnvFrom:      []string{},
+		EnvFrom:         []string{},
-		EnvValueFrom: []string{},
+		EnvValueFrom:    []string{},
-		Mount:        []string{},
+		Mount:           []string{},
-		Volume:       []string{},
+		Volume:          []string{},
-		Arg:          []string{},
+		Arg:             []string{},
-		Command:      []string{},
+		Command:         []string{},
+		SecurityContext: "strict",
 	}
 	flags := &PodSpecFlags{}
 	testCmd := &cobra.Command{
@@ -86,12 +86,12 @@ func TestPodSpecResolve(t *testing.T) {
 					},
 				},
 				ReadinessProbe: &corev1.Probe{
-					ProbeHandler: v1.ProbeHandler{
+					ProbeHandler: corev1.ProbeHandler{
 						HTTPGet: &corev1.HTTPGetAction{Port: intstr.Parse("8080"), Path: "/path"},
 					},
 				},
 				LivenessProbe: &corev1.Probe{
-					ProbeHandler: v1.ProbeHandler{
+					ProbeHandler: corev1.ProbeHandler{
 						HTTPGet: &corev1.HTTPGetAction{Port: intstr.Parse("8080"), Path: "/path"},
 					},
 				},
@@ -223,6 +223,7 @@ containers:
 					Limits:   corev1.ResourceList{},
 					Requests: corev1.ResourceList{},
 				},
+				SecurityContext: DefaultStrictSecCon(),
 			},
 			{
 				Name:  "foo",
@@ -393,10 +394,11 @@ func TestPodSpecResolveWithEnvFile(t *testing.T) {
 					},
 				},
 				Env: []corev1.EnvVar{{Name: "svcOwner", Value: "James"}, {Name: "svcAuthor", Value: "James"}},
-				Resources: v1.ResourceRequirements{
+				Resources: corev1.ResourceRequirements{
-					Limits:   v1.ResourceList{},
+					Limits:   corev1.ResourceList{},
-					Requests: v1.ResourceList{},
+					Requests: corev1.ResourceList{},
 				},
+				SecurityContext: DefaultStrictSecCon(),
 			},
 		},
 	}

test/e2e/service_export_test.go

@@ -53,7 +53,7 @@ func TestServiceExport(t *testing.T) {
 	defer r.DumpIfFailed()
 
 	t.Log("create service with byo revision")
-	serviceCreateWithOptions(r, "hello", "--revision-name", "rev1")
+	serviceCreateWithOptions(r, "hello", "--revision-name", "rev1", "--security-context=none")
 
 	userImage := pkgtest.ImagePath("helloworld")
 	if strings.Contains(userImage, "@") {
@@ -233,7 +233,7 @@ func TestServiceExport(t *testing.T) {
 	), "--with-revisions", "--mode", "export", "-o", "yaml")
 
 	t.Log("create and export service 'foo' and verify that serviceUID and configurationUID labels are absent")
-	serviceCreateWithOptions(r, "foo")
+	serviceCreateWithOptions(r, "foo", "--security-context=none")
 	output := serviceExportOutput(r, "foo", "-o", "json")
 	actSvc := servingv1.Service{}
 	err = json.Unmarshal([]byte(output), &actSvc)