From b3cf17344dc21d8359b56f4419dbc7414cfeb857 Mon Sep 17 00:00:00 2001 From: Stavros Kontopoulos Date: Fri, 16 Sep 2022 17:28:48 +0300 Subject: [PATCH 1/3] upgrade from deprecated apis --- openshift-knative-operator/pkg/common/api.go | 89 +++++++++++++++++++ .../pkg/common/api_test.go | 76 ++++++++++++++++ .../pkg/eventing/extension.go | 5 +- .../pkg/serving/extension.go | 49 ++-------- .../pkg/serving/extension_test.go | 69 -------------- .../kafka/kafka_source_to_ksvc_test.go | 48 ++++++++-- 6 files changed, 215 insertions(+), 121 deletions(-) create mode 100644 openshift-knative-operator/pkg/common/api.go create mode 100644 openshift-knative-operator/pkg/common/api_test.go diff --git a/openshift-knative-operator/pkg/common/api.go b/openshift-knative-operator/pkg/common/api.go new file mode 100644 index 0000000000..4fd97908b3 --- /dev/null +++ b/openshift-knative-operator/pkg/common/api.go @@ -0,0 +1,89 @@ +package common + +import ( + "fmt" + "strings" + + "github.com/blang/semver/v4" + mf "github.com/manifestival/manifestival" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/client-go/discovery" +) + +// UpgradePodDisruptionBudget upgrade the API version to policy/v1 +func UpgradePodDisruptionBudget() mf.Transformer { + return func(u *unstructured.Unstructured) error { + if u.GetKind() != "PodDisruptionBudget" { + return nil + } + if u.GetAPIVersion() != "policy/v1beta1" { + return nil + } + u.SetAPIVersion("policy/v1") + return nil + } +} + +// UpgradeHorizontalPodAutoscaler upgrade the API version to autoscaling/v2 +func UpgradeHorizontalPodAutoscaler() mf.Transformer { + return func(u *unstructured.Unstructured) error { + if u.GetKind() != "HorizontalPodAutoscaler" { + return nil + } + if u.GetAPIVersion() != "autoscaling/v2beta2" { + return nil + } + u.SetAPIVersion("autoscaling/v2") + return nil + } +} + +// CheckMinimumVersion checks if the version in the arg meets the requirement or not. +// It is similar logic with CheckMinimumVersion() in knative.dev/pkg/version. +func CheckMinimumVersion(versioner discovery.ServerVersionInterface, version string) error { + v, err := versioner.ServerVersion() + if err != nil { + return err + } + currentVersion, err := semver.Make(normalizeVersion(v.GitVersion)) + if err != nil { + return err + } + + minimumVersion, err := semver.Make(normalizeVersion(version)) + if err != nil { + return err + } + + // If no specific pre-release requirement is set, we default to "-0" to always allow + // pre-release versions of the same Major.Minor.Patch version. + if len(minimumVersion.Pre) == 0 { + minimumVersion.Pre = []semver.PRVersion{{VersionNum: 0, IsNum: true}} + } + + if currentVersion.LT(minimumVersion) { + return fmt.Errorf("kubernetes version %q is not compatible, need at least %q", + currentVersion, minimumVersion) + } + return nil +} + +func DeprecatedAPIsTranformers(d discovery.DiscoveryInterface) []mf.Transformer { + transformers := []mf.Transformer{} + // Enforce the new version, try to upgrade existing resources. + // The policy/v1beta1 API version of PodDisruptionBudget will no longer be served in v1.25. + // The autoscaling/v2beta2 API version of HorizontalPodAutoscaler will no longer be served in v1.26 + // TODO: When we move away from releases that bring v1beta1 we can remove this part + if err := CheckMinimumVersion(d, "1.25.0"); err == nil { + transformers = append(transformers, UpgradePodDisruptionBudget(), UpgradeHorizontalPodAutoscaler()) + } + return transformers +} + +func normalizeVersion(v string) string { + if strings.HasPrefix(v, "v") { + // No need to account for unicode widths. + return v[1:] + } + return v +} diff --git a/openshift-knative-operator/pkg/common/api_test.go b/openshift-knative-operator/pkg/common/api_test.go new file mode 100644 index 0000000000..059563960b --- /dev/null +++ b/openshift-knative-operator/pkg/common/api_test.go @@ -0,0 +1,76 @@ +package common + +import ( + "errors" + "testing" + + "k8s.io/apimachinery/pkg/version" +) + +type testVersioner struct { + version string + err error +} + +func (t *testVersioner) ServerVersion() (*version.Info, error) { + return &version.Info{GitVersion: t.version}, t.err +} + +func TestVersionCheck(t *testing.T) { + tests := []struct { + name string + actualVersion *testVersioner + wantError bool + }{{ + name: "greater version (patch)", + actualVersion: &testVersioner{version: "v1.20.0"}, + }, { + name: "greater version (patch), no v", + actualVersion: &testVersioner{version: "1.20.0"}, + }, { + name: "greater version (patch), pre-release", + actualVersion: &testVersioner{version: "1.20.2-kpn-065dce"}, + }, { + name: "greater version (patch), pre-release with build", + actualVersion: &testVersioner{version: "1.20.0-1095+9689d22dc3121e-dirty"}, + }, { + name: "greater version (minor)", + actualVersion: &testVersioner{version: "v1.20.0"}, + }, { + name: "same version", + actualVersion: &testVersioner{version: "v1.20.0"}, + }, { + name: "same version with build", + actualVersion: &testVersioner{version: "v1.20.0+k3s.1"}, + }, { + name: "same version with pre-release", + actualVersion: &testVersioner{version: "v1.20.0-k3s.1"}, + }, { + name: "smaller version", + actualVersion: &testVersioner{version: "v1.19.3"}, + wantError: true, + }, { + name: "error while fetching", + actualVersion: &testVersioner{err: errors.New("random error")}, + wantError: true, + }, { + name: "unparseable actual version", + actualVersion: &testVersioner{version: "v1.19.foo"}, + wantError: true, + }} + + minVersion := "1.20.0" + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + err := CheckMinimumVersion(test.actualVersion, minVersion) + if err == nil && test.wantError { + t.Errorf("Expected an error for minimum: %q, actual: %v", minVersion, test.actualVersion) + } + + if err != nil && !test.wantError { + t.Errorf("Expected no error but got %v for minimum: %q, actual: %v", err, minVersion, test.actualVersion) + } + }) + } +} diff --git a/openshift-knative-operator/pkg/eventing/extension.go b/openshift-knative-operator/pkg/eventing/extension.go index b1a70bb1d7..03f6fb68b4 100644 --- a/openshift-knative-operator/pkg/eventing/extension.go +++ b/openshift-knative-operator/pkg/eventing/extension.go @@ -34,8 +34,9 @@ func (e *extension) Manifests(ke operatorv1alpha1.KComponent) ([]mf.Manifest, er } func (e *extension) Transformers(ke operatorv1alpha1.KComponent) []mf.Transformer { - return append([]mf.Transformer{common.InjectCommonLabelIntoNamespace(), common.VersionedJobNameTransform()}, - monitoring.GetEventingTransformers(ke)...) + tf := []mf.Transformer{common.InjectCommonLabelIntoNamespace(), common.VersionedJobNameTransform()} + tf = append(tf, common.DeprecatedAPIsTranformers(e.kubeclient.Discovery())...) + return append(tf, monitoring.GetEventingTransformers(ke)...) } func (e *extension) Reconcile(ctx context.Context, comp operatorv1alpha1.KComponent) error { diff --git a/openshift-knative-operator/pkg/serving/extension.go b/openshift-knative-operator/pkg/serving/extension.go index 95efc6488b..1685169236 100644 --- a/openshift-knative-operator/pkg/serving/extension.go +++ b/openshift-knative-operator/pkg/serving/extension.go @@ -4,9 +4,7 @@ import ( "context" "fmt" "os" - "strings" - "github.com/blang/semver/v4" mf "github.com/manifestival/manifestival" "github.com/openshift-knative/serverless-operator/openshift-knative-operator/pkg/common" "github.com/openshift-knative/serverless-operator/openshift-knative-operator/pkg/monitoring" @@ -17,7 +15,6 @@ import ( apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/client-go/discovery" "k8s.io/client-go/kubernetes" "k8s.io/client-go/tools/cache" operatorv1alpha1 "knative.dev/operator/pkg/apis/operator/v1alpha1" @@ -63,7 +60,7 @@ func (e *extension) Manifests(ks operatorv1alpha1.KComponent) ([]mf.Manifest, er } func (e *extension) Transformers(ks operatorv1alpha1.KComponent) []mf.Transformer { - return append([]mf.Transformer{ + tf := []mf.Transformer{ common.InjectCommonLabelIntoNamespace(), common.InjectEnvironmentIntoDeployment("controller", "controller", corev1.EnvVar{Name: "HTTP_PROXY", Value: os.Getenv("HTTP_PROXY")}, @@ -74,7 +71,9 @@ func (e *extension) Transformers(ks operatorv1alpha1.KComponent) []mf.Transforme addKourierEnvValues(ks), enableSecretInformerFiltering(ks), common.VersionedJobNameTransform(), - }, monitoring.GetServingTransformers(ks)...) + } + tf = append(tf, common.DeprecatedAPIsTranformers(e.kubeclient.Discovery())...) + return append(tf, monitoring.GetServingTransformers(ks)...) } func (e *extension) Reconcile(ctx context.Context, comp operatorv1alpha1.KComponent) error { @@ -126,7 +125,7 @@ func (e *extension) Reconcile(ctx context.Context, comp operatorv1alpha1.KCompon // Changing service type from LoadBalancer to ClusterIP has a bug https://github.com/kubernetes/kubernetes/pull/95196 // Do not apply the default if the version is less than v1.20.0. - if err := checkMinimumVersion(e.kubeclient.Discovery(), "1.20.0"); err != nil { + if err := common.CheckMinimumVersion(e.kubeclient.Discovery(), "1.20.0"); err != nil { log.Warnf("Could not apply default service type for Kourier Gateway: %v", err) } else { // Apply Kourier gateway service type. @@ -197,41 +196,3 @@ func (e *extension) fetchLoggingHost(ctx context.Context) string { } return route.Status.Ingress[0].Host } - -// checkMinimumVersion checks if the version in the arg meets the requirement or not. -// It is similar logic with CheckMinimumVersion() in knative.dev/pkg/version. -func checkMinimumVersion(versioner discovery.ServerVersionInterface, version string) error { - v, err := versioner.ServerVersion() - if err != nil { - return err - } - currentVersion, err := semver.Make(normalizeVersion(v.GitVersion)) - if err != nil { - return err - } - - minimumVersion, err := semver.Make(normalizeVersion(version)) - if err != nil { - return err - } - - // If no specific pre-release requirement is set, we default to "-0" to always allow - // pre-release versions of the same Major.Minor.Patch version. - if len(minimumVersion.Pre) == 0 { - minimumVersion.Pre = []semver.PRVersion{{VersionNum: 0, IsNum: true}} - } - - if currentVersion.LT(minimumVersion) { - return fmt.Errorf("kubernetes version %q is not compatible, need at least %q", - currentVersion, minimumVersion) - } - return nil -} - -func normalizeVersion(v string) string { - if strings.HasPrefix(v, "v") { - // No need to account for unicode widths. - return v[1:] - } - return v -} diff --git a/openshift-knative-operator/pkg/serving/extension_test.go b/openshift-knative-operator/pkg/serving/extension_test.go index 934363314d..ad8fa499ca 100644 --- a/openshift-knative-operator/pkg/serving/extension_test.go +++ b/openshift-knative-operator/pkg/serving/extension_test.go @@ -2,7 +2,6 @@ package serving import ( "context" - "errors" "fmt" "os" "strconv" @@ -555,71 +554,3 @@ func ks(mods ...func(*operatorv1alpha1.KnativeServing)) *operatorv1alpha1.Knativ return base } - -type testVersioner struct { - version string - err error -} - -func (t *testVersioner) ServerVersion() (*version.Info, error) { - return &version.Info{GitVersion: t.version}, t.err -} - -func TestVersionCheck(t *testing.T) { - tests := []struct { - name string - actualVersion *testVersioner - wantError bool - }{{ - name: "greater version (patch)", - actualVersion: &testVersioner{version: "v1.20.0"}, - }, { - name: "greater version (patch), no v", - actualVersion: &testVersioner{version: "1.20.0"}, - }, { - name: "greater version (patch), pre-release", - actualVersion: &testVersioner{version: "1.20.2-kpn-065dce"}, - }, { - name: "greater version (patch), pre-release with build", - actualVersion: &testVersioner{version: "1.20.0-1095+9689d22dc3121e-dirty"}, - }, { - name: "greater version (minor)", - actualVersion: &testVersioner{version: "v1.20.0"}, - }, { - name: "same version", - actualVersion: &testVersioner{version: "v1.20.0"}, - }, { - name: "same version with build", - actualVersion: &testVersioner{version: "v1.20.0+k3s.1"}, - }, { - name: "same version with pre-release", - actualVersion: &testVersioner{version: "v1.20.0-k3s.1"}, - }, { - name: "smaller version", - actualVersion: &testVersioner{version: "v1.19.3"}, - wantError: true, - }, { - name: "error while fetching", - actualVersion: &testVersioner{err: errors.New("random error")}, - wantError: true, - }, { - name: "unparseable actual version", - actualVersion: &testVersioner{version: "v1.19.foo"}, - wantError: true, - }} - - minVersion := "1.20.0" - - for _, test := range tests { - t.Run(test.name, func(t *testing.T) { - err := checkMinimumVersion(test.actualVersion, minVersion) - if err == nil && test.wantError { - t.Errorf("Expected an error for minimum: %q, actual: %v", minVersion, test.actualVersion) - } - - if err != nil && !test.wantError { - t.Errorf("Expected no error but got %v for minimum: %q, actual: %v", err, minVersion, test.actualVersion) - } - }) - } -} diff --git a/test/extensione2e/kafka/kafka_source_to_ksvc_test.go b/test/extensione2e/kafka/kafka_source_to_ksvc_test.go index 849edfda2b..7b5cb41610 100644 --- a/test/extensione2e/kafka/kafka_source_to_ksvc_test.go +++ b/test/extensione2e/kafka/kafka_source_to_ksvc_test.go @@ -20,6 +20,7 @@ import ( kafkasourcev1beta1 "knative.dev/eventing-kafka/pkg/apis/sources/v1beta1" duckv1 "knative.dev/pkg/apis/duck/v1" + "github.com/openshift-knative/serverless-operator/openshift-knative-operator/pkg/common" "github.com/openshift-knative/serverless-operator/test" "github.com/openshift-knative/serverless-operator/test/servinge2e" ) @@ -49,7 +50,7 @@ var ( kafkaGVR = schema.GroupVersionResource{Group: "kafka.strimzi.io", Version: "v1beta1", Resource: "kafkatopics"} ) -func createCronJobObj(name, topic, server string) *batchv1beta1.CronJob { +func createCronJobObjV1Beta1(name, topic, server string) *batchv1beta1.CronJob { return &batchv1beta1.CronJob{ ObjectMeta: metav1.ObjectMeta{ Name: name, @@ -77,6 +78,34 @@ func createCronJobObj(name, topic, server string) *batchv1beta1.CronJob { } } +func createCronJobObjV1(name, topic, server string) *batchv1.CronJob { + return &batchv1.CronJob{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: test.Namespace, + }, + Spec: batchv1.CronJobSpec{ + Schedule: "* * * * *", + JobTemplate: batchv1.JobTemplateSpec{ + Spec: batchv1.JobSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "kafka-message-test", + Image: "strimzi/kafka:0.16.2-kafka-2.4.0", + Command: []string{"sh", "-c", fmt.Sprintf(`echo "%s" | bin/kafka-console-producer.sh --broker-list %s --topic %s`, helloWorldText, server, topic)}, + }, + }, + RestartPolicy: corev1.RestartPolicyOnFailure, + }, + }, + }, + }, + }, + } +} + func createKafkaSourceObj(sourceName, sinkName, topicName string, auth kafkabindingv1beta1.KafkaAuthSpec) kafkasourcev1beta1.KafkaSource { return kafkasourcev1beta1.KafkaSource{ ObjectMeta: metav1.ObjectMeta{ @@ -262,12 +291,19 @@ func TestKafkaSourceToKnativeService(t *testing.T) { } // send event to kafka topic - cj := createCronJobObj(cronJobName+"-"+name, kafkaTopicName+"-"+name, kafkaSource.Spec.BootstrapServers[0]) - _, err = client.Clients.Kube.BatchV1beta1().CronJobs(test.Namespace).Create(context.Background(), cj, metav1.CreateOptions{}) - if err != nil { - t.Fatalf("Unable to create batch cronjob(%s): %v", cj.GetName(), err) + if err := common.CheckMinimumVersion(client.Clients.Kube.Discovery(), "1.25.0"); err == nil { + cj := createCronJobObjV1(cronJobName+"-"+name, kafkaTopicName+"-"+name, kafkaSource.Spec.BootstrapServers[0]) + _, err = client.Clients.Kube.BatchV1().CronJobs(test.Namespace).Create(context.Background(), cj, metav1.CreateOptions{}) + if err != nil { + t.Fatalf("Unable to create batch cronjob(%s): %v", cj.GetName(), err) + } + } else { + cj := createCronJobObjV1Beta1(cronJobName+"-"+name, kafkaTopicName+"-"+name, kafkaSource.Spec.BootstrapServers[0]) + _, err = client.Clients.Kube.BatchV1beta1().CronJobs(test.Namespace).Create(context.Background(), cj, metav1.CreateOptions{}) + if err != nil { + t.Fatalf("Unable to create batch cronjob(%s): %v", cj.GetName(), err) + } } - servinge2e.WaitForRouteServingText(t, client, ksvc.Status.URL.URL(), helloWorldText) } } From e734e1c4a9a2e8d1de78e825f7a3488a35d74a7d Mon Sep 17 00:00:00 2001 From: Stavros Kontopoulos Date: Mon, 19 Sep 2022 11:17:35 +0300 Subject: [PATCH 2/3] psec --- hack/patches/006-version.patch | 38 +++++++ .../knativekafka/knativekafka_controller.go | 7 +- ...erless-operator.clusterserviceversion.yaml | 20 +++- openshift-knative-operator/pkg/common/api.go | 102 +++++++++++++++-- .../pkg/common/api_test.go | 107 ++++++++++++++++++ .../pkg/eventing/extension.go | 4 +- .../pkg/monitoring/rbac_proxy_injection.go | 8 ++ .../monitoring/rbac_proxy_injection_test.go | 8 ++ .../pkg/serving/extension.go | 4 +- templates/csv.yaml | 21 +++- .../kafka/kafka_source_to_ksvc_test.go | 2 +- .../pkg/environment/client_config.go | 7 +- .../pkg/client/config/config.go | 9 +- 13 files changed, 309 insertions(+), 28 deletions(-) create mode 100644 hack/patches/006-version.patch diff --git a/hack/patches/006-version.patch b/hack/patches/006-version.patch new file mode 100644 index 0000000000..8897810a98 --- /dev/null +++ b/hack/patches/006-version.patch @@ -0,0 +1,38 @@ +diff --git a/vendor/knative.dev/pkg/environment/client_config.go b/vendor/knative.dev/pkg/environment/client_config.go +index 04d4220b..d094bf0c 100644 +--- a/vendor/knative.dev/pkg/environment/client_config.go ++++ b/vendor/knative.dev/pkg/environment/client_config.go +@@ -42,9 +42,10 @@ func (c *ClientConfig) InitFlags(fs *flag.FlagSet) { + fs.StringVar(&c.ServerURL, "server", "", + "The address of the Kubernetes API server. Overrides any value in kubeconfig. Only required if out-of-cluster.") + +- fs.StringVar(&c.Kubeconfig, "kubeconfig", os.Getenv("KUBECONFIG"), +- "Path to a kubeconfig. Only required if out-of-cluster.") +- ++ if fs.Lookup("kubeconfig") == nil { ++ fs.StringVar(&c.Kubeconfig, "kubeconfig", os.Getenv("KUBECONFIG"), ++ "Path to a kubeconfig. Only required if out-of-cluster.") ++ } + fs.IntVar(&c.Burst, "kube-api-burst", 0, "Maximum burst for throttle.") + + fs.Float64Var(&c.QPS, "kube-api-qps", 0, "Maximum QPS to the server from the client.") +diff --git a/vendor/sigs.k8s.io/controller-runtime/pkg/client/config/config.go b/vendor/sigs.k8s.io/controller-runtime/pkg/client/config/config.go +index 235a7e45..d4110dc9 100644 +--- a/vendor/sigs.k8s.io/controller-runtime/pkg/client/config/config.go ++++ b/vendor/sigs.k8s.io/controller-runtime/pkg/client/config/config.go +@@ -35,9 +35,12 @@ var ( + ) + + func init() { +- // TODO: Fix this to allow double vendoring this library but still register flags on behalf of users +- flag.StringVar(&kubeconfig, "kubeconfig", "", +- "Paths to a kubeconfig. Only required if out-of-cluster.") ++ // Need to avoid conflict with knative sharedmain kubeconfig parsing ++ // For more check here: https://github.com/kubernetes-sigs/controller-runtime/issues/878 ++ if flag.Lookup("kubeconfig") == nil { ++ flag.StringVar(&kubeconfig, "kubeconfig", "", ++ "Paths to a kubeconfig. Only required if out-of-cluster.") ++ } + } + + // GetConfig creates a *rest.Config for talking to a Kubernetes API server. diff --git a/knative-operator/pkg/controller/knativekafka/knativekafka_controller.go b/knative-operator/pkg/controller/knativekafka/knativekafka_controller.go index 60b9f5b5e0..3914f92ed7 100644 --- a/knative-operator/pkg/controller/knativekafka/knativekafka_controller.go +++ b/knative-operator/pkg/controller/knativekafka/knativekafka_controller.go @@ -305,7 +305,8 @@ func (r *ReconcileKnativeKafka) transform(manifest *mf.Manifest, instance *serve return err } } - m, err := manifest.Transform( + tfs := []mf.Transformer{} + tfs = append(append(tfs, mf.InjectOwner(instance), common.SetAnnotations(map[string]string{ common.KafkaOwnerName: instance.Name, @@ -317,8 +318,8 @@ func (r *ReconcileKnativeKafka) transform(manifest *mf.Manifest, instance *serve ImageTransform(common.BuildImageOverrideMapFromEnviron(os.Environ(), "KAFKA_IMAGE_")), socommon.VersionedJobNameTransform(), socommon.ConfigMapVolumeChecksumTransform(context.Background(), r.client, sets.NewString("config-tracing", "kafka-config-logging")), - rbacProxyTranform, - ) + rbacProxyTranform), socommon.DeprecatedAPIsTranformersFromConfig()...) + m, err := manifest.Transform(tfs...) if err != nil { return fmt.Errorf("failed to transform manifest: %w", err) } diff --git a/olm-catalog/serverless-operator/manifests/serverless-operator.clusterserviceversion.yaml b/olm-catalog/serverless-operator/manifests/serverless-operator.clusterserviceversion.yaml index d44fc81e82..29a65a834b 100644 --- a/olm-catalog/serverless-operator/manifests/serverless-operator.clusterserviceversion.yaml +++ b/olm-catalog/serverless-operator/manifests/serverless-operator.clusterserviceversion.yaml @@ -492,7 +492,9 @@ spec: runAsNonRoot: true capabilities: drop: - - all + - ALL + seccompProfile: + type: RuntimeDefault # Openshift specific extensions in the "old" operator format. Ships KnativeKafka and # other Openshift specifica that have not yet been moved to the operator above. - name: knative-openshift @@ -515,6 +517,14 @@ spec: volumeMounts: - mountPath: /cli-artifacts name: cli-artifacts + securityContext: + allowPrivilegeEscalation: false + runAsNonRoot: true + capabilities: + drop: + - ALL + seccompProfile: + type: RuntimeDefault containers: - name: knative-openshift # This reference will be replaced in local builds and CI via hack/lib/catalogsource.bash. @@ -657,7 +667,9 @@ spec: runAsNonRoot: true capabilities: drop: - - all + - ALL + seccompProfile: + type: RuntimeDefault volumes: - name: cli-artifacts emptyDir: {} @@ -703,7 +715,9 @@ spec: runAsNonRoot: true capabilities: drop: - - all + - ALL + seccompProfile: + type: RuntimeDefault webhookdefinitions: - generateName: validating.knativeeventings.operator.serverless.openshift.io type: ValidatingAdmissionWebhook diff --git a/openshift-knative-operator/pkg/common/api.go b/openshift-knative-operator/pkg/common/api.go index 4fd97908b3..cf0a4b7f12 100644 --- a/openshift-knative-operator/pkg/common/api.go +++ b/openshift-knative-operator/pkg/common/api.go @@ -2,12 +2,22 @@ package common import ( "fmt" + "strings" - "github.com/blang/semver/v4" - mf "github.com/manifestival/manifestival" + batchv1 "k8s.io/api/batch/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/client-go/discovery" + "k8s.io/client-go/kubernetes" + "k8s.io/client-go/kubernetes/scheme" + + "github.com/blang/semver/v4" + mf "github.com/manifestival/manifestival" + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" + + "knative.dev/pkg/ptr" + "sigs.k8s.io/controller-runtime/pkg/client/config" ) // UpgradePodDisruptionBudget upgrade the API version to policy/v1 @@ -38,8 +48,72 @@ func UpgradeHorizontalPodAutoscaler() mf.Transformer { } } -// CheckMinimumVersion checks if the version in the arg meets the requirement or not. -// It is similar logic with CheckMinimumVersion() in knative.dev/pkg/version. +// SetSecurityContextForAdmissionController set the required pod security context to avoid issues on K8s 1.25+. +// For more check: https://connect.redhat.com/en/blog/important-openshift-changes-pod-security-standards +func SetSecurityContextForAdmissionController() mf.Transformer { + return func(u *unstructured.Unstructured) error { + switch u.GetKind() { + case "Deployment": + deployment := &appsv1.Deployment{} + if err := scheme.Scheme.Convert(u, deployment, nil); err != nil { + return fmt.Errorf("failed to convert Unstructured to Deployment: %w", err) + } + obj := deployment + podSpec := &deployment.Spec.Template.Spec + containers := podSpec.Containers + for i := range containers { + setPodSecurityContext(&containers[i]) + } + if err := scheme.Scheme.Convert(obj, u, nil); err != nil { + return err + } + case "Job": + job := &batchv1.Job{} + if err := scheme.Scheme.Convert(u, job, nil); err != nil { + return fmt.Errorf("failed to convert Unstructured to Job: %w", err) + } + obj := job + podSpec := &job.Spec.Template.Spec + containers := podSpec.Containers + for i := range containers { + setPodSecurityContext(&containers[i]) + } + if err := scheme.Scheme.Convert(obj, u, nil); err != nil { + return err + } + } + return nil + } +} + +func setPodSecurityContext(container *corev1.Container) { + if container.SecurityContext == nil { + container.SecurityContext = &corev1.SecurityContext{ + AllowPrivilegeEscalation: ptr.Bool(false), + ReadOnlyRootFilesystem: ptr.Bool(true), + RunAsNonRoot: ptr.Bool(true), + Capabilities: &corev1.Capabilities{Drop: []corev1.Capability{"ALL"}}, + SeccompProfile: &corev1.SeccompProfile{Type: corev1.SeccompProfileTypeRuntimeDefault}, + } + } else { + if container.SecurityContext.RunAsNonRoot == nil { + container.SecurityContext.RunAsNonRoot = ptr.Bool(true) + } + if container.SecurityContext.ReadOnlyRootFilesystem == nil { + container.SecurityContext.ReadOnlyRootFilesystem = ptr.Bool(true) + } + if container.SecurityContext.AllowPrivilegeEscalation == nil { + container.SecurityContext.AllowPrivilegeEscalation = ptr.Bool(false) + } + container.SecurityContext.Capabilities = &corev1.Capabilities{Drop: []corev1.Capability{"ALL"}} + if container.SecurityContext.SeccompProfile == nil { + container.SecurityContext.SeccompProfile = &corev1.SeccompProfile{Type: corev1.SeccompProfileTypeRuntimeDefault} + } + } +} + +// CheckMinimumVersion checks if current K8s version we are on is higher than the one passed. +// If an error is returned then we func CheckMinimumVersion(versioner discovery.ServerVersionInterface, version string) error { v, err := versioner.ServerVersion() if err != nil { @@ -68,14 +142,28 @@ func CheckMinimumVersion(versioner discovery.ServerVersionInterface, version str return nil } +// DeprecatedAPIsTranformersFromConfig check if we are on the right K8s version and return +// the related transformers. Meant to be used by the knative-openshift operator which uses controller runtime +// and for which we need to construct the discovery value. +func DeprecatedAPIsTranformersFromConfig() []mf.Transformer { + cfg, err := config.GetConfig() + if err != nil { + panic(err) + } + clset := kubernetes.NewForConfigOrDie(cfg) + return DeprecatedAPIsTranformers(clset.Discovery()) +} + +// DeprecatedAPIsTranformers check if we are on the right K8s version and return +// the related transformers. func DeprecatedAPIsTranformers(d discovery.DiscoveryInterface) []mf.Transformer { transformers := []mf.Transformer{} - // Enforce the new version, try to upgrade existing resources. + // Enforce the new version, try to upgrade existing resources for 4.11 to also avoid warnings. // The policy/v1beta1 API version of PodDisruptionBudget will no longer be served in v1.25. // The autoscaling/v2beta2 API version of HorizontalPodAutoscaler will no longer be served in v1.26 // TODO: When we move away from releases that bring v1beta1 we can remove this part - if err := CheckMinimumVersion(d, "1.25.0"); err == nil { - transformers = append(transformers, UpgradePodDisruptionBudget(), UpgradeHorizontalPodAutoscaler()) + if err := CheckMinimumVersion(d, "1.24.0"); err == nil { + transformers = append(transformers, UpgradePodDisruptionBudget(), UpgradeHorizontalPodAutoscaler(), SetSecurityContextForAdmissionController()) } return transformers } diff --git a/openshift-knative-operator/pkg/common/api_test.go b/openshift-knative-operator/pkg/common/api_test.go index 059563960b..2575842574 100644 --- a/openshift-knative-operator/pkg/common/api_test.go +++ b/openshift-knative-operator/pkg/common/api_test.go @@ -4,7 +4,14 @@ import ( "errors" "testing" + "github.com/google/go-cmp/cmp" + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/version" + "k8s.io/client-go/kubernetes/scheme" + "knative.dev/pkg/ptr" ) type testVersioner struct { @@ -74,3 +81,103 @@ func TestVersionCheck(t *testing.T) { }) } } + +func TestPodSecurityContext(t *testing.T) { + spec := func(containers ...corev1.Container) appsv1.DeploymentSpec { + return appsv1.DeploymentSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: containers, + }, + }, + } + } + + tests := []struct { + name string + in *appsv1.Deployment + deployment string + container string + envs []corev1.EnvVar + want *appsv1.Deployment + }{{ + name: "ignore", + deployment: "foo", + container: "container1", + in: &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + }, + Spec: spec(corev1.Container{ + Name: "container1", + }), + }, + want: &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + }, + Spec: spec(corev1.Container{ + Name: "container1", + SecurityContext: &corev1.SecurityContext{ + AllowPrivilegeEscalation: ptr.Bool(false), + ReadOnlyRootFilesystem: ptr.Bool(true), + RunAsNonRoot: ptr.Bool(true), + Capabilities: &corev1.Capabilities{Drop: []corev1.Capability{"ALL"}}, + SeccompProfile: &corev1.SeccompProfile{Type: corev1.SeccompProfileTypeRuntimeDefault}, + }, + }), + }, + }, { + name: "ignore", + deployment: "foo", + container: "container1", + in: &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + }, + Spec: spec(corev1.Container{ + Name: "container1", + SecurityContext: &corev1.SecurityContext{ + ReadOnlyRootFilesystem: ptr.Bool(false), + }, + }), + }, + want: &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + }, + Spec: spec(corev1.Container{ + Name: "container1", + SecurityContext: &corev1.SecurityContext{ + AllowPrivilegeEscalation: ptr.Bool(false), + ReadOnlyRootFilesystem: ptr.Bool(false), + RunAsNonRoot: ptr.Bool(true), + Capabilities: &corev1.Capabilities{Drop: []corev1.Capability{"ALL"}}, + SeccompProfile: &corev1.SeccompProfile{Type: corev1.SeccompProfileTypeRuntimeDefault}, + }, + }), + }, + }} + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + u := &unstructured.Unstructured{} + if err := scheme.Scheme.Convert(test.in, u, nil); err != nil { + t.Fatal("Failed to convert deployment to unstructured", err) + } + + if err := SetSecurityContextForAdmissionController()(u); err != nil { + t.Fatal("Unexpected error from transformer", err) + } + + got := &appsv1.Deployment{} + if err := scheme.Scheme.Convert(u, got, nil); err != nil { + t.Fatal("Failed to convert unstructured to deployment", err) + } + + if !cmp.Equal(got, test.want) { + t.Errorf("Got = %v, want: %v, diff:\n%s", got, test.want, cmp.Diff(got, test.want)) + } + }) + } +} diff --git a/openshift-knative-operator/pkg/eventing/extension.go b/openshift-knative-operator/pkg/eventing/extension.go index 03f6fb68b4..924d5c4741 100644 --- a/openshift-knative-operator/pkg/eventing/extension.go +++ b/openshift-knative-operator/pkg/eventing/extension.go @@ -35,8 +35,8 @@ func (e *extension) Manifests(ke operatorv1alpha1.KComponent) ([]mf.Manifest, er func (e *extension) Transformers(ke operatorv1alpha1.KComponent) []mf.Transformer { tf := []mf.Transformer{common.InjectCommonLabelIntoNamespace(), common.VersionedJobNameTransform()} - tf = append(tf, common.DeprecatedAPIsTranformers(e.kubeclient.Discovery())...) - return append(tf, monitoring.GetEventingTransformers(ke)...) + tf = append(tf, monitoring.GetEventingTransformers(ke)...) + return append(tf, common.DeprecatedAPIsTranformers(e.kubeclient.Discovery())...) } func (e *extension) Reconcile(ctx context.Context, comp operatorv1alpha1.KComponent) error { diff --git a/openshift-knative-operator/pkg/monitoring/rbac_proxy_injection.go b/openshift-knative-operator/pkg/monitoring/rbac_proxy_injection.go index bfef48cfc1..8e420679a6 100644 --- a/openshift-knative-operator/pkg/monitoring/rbac_proxy_injection.go +++ b/openshift-knative-operator/pkg/monitoring/rbac_proxy_injection.go @@ -12,6 +12,7 @@ import ( "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/client-go/kubernetes/scheme" + "knative.dev/pkg/ptr" ) const ( @@ -82,6 +83,13 @@ func InjectRbacProxyContainer(deployments sets.String) mf.Transformer { "--v=10", }, } + rbacProxyContainer.SecurityContext = &corev1.SecurityContext{ + AllowPrivilegeEscalation: ptr.Bool(false), + ReadOnlyRootFilesystem: ptr.Bool(true), + RunAsNonRoot: ptr.Bool(true), + Capabilities: &corev1.Capabilities{Drop: []corev1.Capability{"ALL"}}, + SeccompProfile: &corev1.SeccompProfile{Type: corev1.SeccompProfileTypeRuntimeDefault}, + } podSpec.Containers = append(podSpec.Containers, rbacProxyContainer) podSpec.Volumes = append(podSpec.Volumes, corev1.Volume{ Name: volumeName, diff --git a/openshift-knative-operator/pkg/monitoring/rbac_proxy_injection_test.go b/openshift-knative-operator/pkg/monitoring/rbac_proxy_injection_test.go index eb088278c5..c768982940 100644 --- a/openshift-knative-operator/pkg/monitoring/rbac_proxy_injection_test.go +++ b/openshift-knative-operator/pkg/monitoring/rbac_proxy_injection_test.go @@ -13,6 +13,7 @@ import ( "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/client-go/kubernetes/scheme" + "knative.dev/pkg/ptr" ) func TestInjectRbacProxyContainerToDeployments(t *testing.T) { @@ -125,6 +126,13 @@ func TestInjectRbacProxyContainerToDeployments(t *testing.T) { "--logtostderr=true", "--v=10", }, + SecurityContext: &corev1.SecurityContext{ + AllowPrivilegeEscalation: ptr.Bool(false), + ReadOnlyRootFilesystem: ptr.Bool(true), + RunAsNonRoot: ptr.Bool(true), + Capabilities: &corev1.Capabilities{Drop: []corev1.Capability{"ALL"}}, + SeccompProfile: &corev1.SeccompProfile{Type: corev1.SeccompProfileTypeRuntimeDefault}, + }, }}, }, }, diff --git a/openshift-knative-operator/pkg/serving/extension.go b/openshift-knative-operator/pkg/serving/extension.go index 1685169236..014543a328 100644 --- a/openshift-knative-operator/pkg/serving/extension.go +++ b/openshift-knative-operator/pkg/serving/extension.go @@ -72,8 +72,8 @@ func (e *extension) Transformers(ks operatorv1alpha1.KComponent) []mf.Transforme enableSecretInformerFiltering(ks), common.VersionedJobNameTransform(), } - tf = append(tf, common.DeprecatedAPIsTranformers(e.kubeclient.Discovery())...) - return append(tf, monitoring.GetServingTransformers(ks)...) + tf = append(tf, monitoring.GetServingTransformers(ks)...) + return append(tf, common.DeprecatedAPIsTranformers(e.kubeclient.Discovery())...) } func (e *extension) Reconcile(ctx context.Context, comp operatorv1alpha1.KComponent) error { diff --git a/templates/csv.yaml b/templates/csv.yaml index ee01c33e47..aabff6382c 100644 --- a/templates/csv.yaml +++ b/templates/csv.yaml @@ -444,8 +444,9 @@ spec: runAsNonRoot: true capabilities: drop: - - all - + - ALL + seccompProfile: + type: RuntimeDefault # Openshift specific extensions in the "old" operator format. Ships KnativeKafka and # other Openshift specifica that have not yet been moved to the operator above. - name: knative-openshift @@ -473,6 +474,14 @@ spec: volumeMounts: - mountPath: /cli-artifacts name: cli-artifacts + securityContext: + allowPrivilegeEscalation: false + runAsNonRoot: true + capabilities: + drop: + - ALL + seccompProfile: + type: RuntimeDefault containers: - name: knative-openshift # This reference will be replaced in local builds and CI via hack/lib/catalogsource.bash. @@ -537,7 +546,9 @@ spec: runAsNonRoot: true capabilities: drop: - - all + - ALL + seccompProfile: + type: RuntimeDefault volumes: - name: cli-artifacts emptyDir: {} @@ -584,7 +595,9 @@ spec: runAsNonRoot: true capabilities: drop: - - all + - ALL + seccompProfile: + type: RuntimeDefault webhookdefinitions: - generateName: validating.knativeeventings.operator.serverless.openshift.io diff --git a/test/extensione2e/kafka/kafka_source_to_ksvc_test.go b/test/extensione2e/kafka/kafka_source_to_ksvc_test.go index 7b5cb41610..e79aa94f18 100644 --- a/test/extensione2e/kafka/kafka_source_to_ksvc_test.go +++ b/test/extensione2e/kafka/kafka_source_to_ksvc_test.go @@ -291,7 +291,7 @@ func TestKafkaSourceToKnativeService(t *testing.T) { } // send event to kafka topic - if err := common.CheckMinimumVersion(client.Clients.Kube.Discovery(), "1.25.0"); err == nil { + if err := common.CheckMinimumVersion(client.Clients.Kube.Discovery(), "1.24.0"); err == nil { cj := createCronJobObjV1(cronJobName+"-"+name, kafkaTopicName+"-"+name, kafkaSource.Spec.BootstrapServers[0]) _, err = client.Clients.Kube.BatchV1().CronJobs(test.Namespace).Create(context.Background(), cj, metav1.CreateOptions{}) if err != nil { diff --git a/vendor/knative.dev/pkg/environment/client_config.go b/vendor/knative.dev/pkg/environment/client_config.go index 04d4220b0a..d094bf0cfa 100644 --- a/vendor/knative.dev/pkg/environment/client_config.go +++ b/vendor/knative.dev/pkg/environment/client_config.go @@ -42,9 +42,10 @@ func (c *ClientConfig) InitFlags(fs *flag.FlagSet) { fs.StringVar(&c.ServerURL, "server", "", "The address of the Kubernetes API server. Overrides any value in kubeconfig. Only required if out-of-cluster.") - fs.StringVar(&c.Kubeconfig, "kubeconfig", os.Getenv("KUBECONFIG"), - "Path to a kubeconfig. Only required if out-of-cluster.") - + if fs.Lookup("kubeconfig") == nil { + fs.StringVar(&c.Kubeconfig, "kubeconfig", os.Getenv("KUBECONFIG"), + "Path to a kubeconfig. Only required if out-of-cluster.") + } fs.IntVar(&c.Burst, "kube-api-burst", 0, "Maximum burst for throttle.") fs.Float64Var(&c.QPS, "kube-api-qps", 0, "Maximum QPS to the server from the client.") diff --git a/vendor/sigs.k8s.io/controller-runtime/pkg/client/config/config.go b/vendor/sigs.k8s.io/controller-runtime/pkg/client/config/config.go index 235a7e450b..2356a0df7b 100644 --- a/vendor/sigs.k8s.io/controller-runtime/pkg/client/config/config.go +++ b/vendor/sigs.k8s.io/controller-runtime/pkg/client/config/config.go @@ -35,9 +35,12 @@ var ( ) func init() { - // TODO: Fix this to allow double vendoring this library but still register flags on behalf of users - flag.StringVar(&kubeconfig, "kubeconfig", "", - "Paths to a kubeconfig. Only required if out-of-cluster.") + // Need to avoid conflict with knative sharedmain kubeconfig parsing + // For more check here: https://github.com/kubernetes-sigs/controller-runtime/issues/878 + if flag.Lookup("kubeconfig") == nil { + flag.StringVar(&kubeconfig, "kubeconfig", "", + "Paths to a kubeconfig. Only required if out-of-cluster.") + } } // GetConfig creates a *rest.Config for talking to a Kubernetes API server. From 0f22fbd436db9bdd83a14c8983741abfb4cadad1 Mon Sep 17 00:00:00 2001 From: Stavros Kontopoulos Date: Mon, 19 Sep 2022 18:45:18 +0300 Subject: [PATCH 3/3] fix rbac proxy --- .../pkg/monitoring/rbac_proxy_injection.go | 8 -------- .../pkg/monitoring/rbac_proxy_injection_test.go | 8 -------- 2 files changed, 16 deletions(-) diff --git a/openshift-knative-operator/pkg/monitoring/rbac_proxy_injection.go b/openshift-knative-operator/pkg/monitoring/rbac_proxy_injection.go index 8e420679a6..bfef48cfc1 100644 --- a/openshift-knative-operator/pkg/monitoring/rbac_proxy_injection.go +++ b/openshift-knative-operator/pkg/monitoring/rbac_proxy_injection.go @@ -12,7 +12,6 @@ import ( "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/client-go/kubernetes/scheme" - "knative.dev/pkg/ptr" ) const ( @@ -83,13 +82,6 @@ func InjectRbacProxyContainer(deployments sets.String) mf.Transformer { "--v=10", }, } - rbacProxyContainer.SecurityContext = &corev1.SecurityContext{ - AllowPrivilegeEscalation: ptr.Bool(false), - ReadOnlyRootFilesystem: ptr.Bool(true), - RunAsNonRoot: ptr.Bool(true), - Capabilities: &corev1.Capabilities{Drop: []corev1.Capability{"ALL"}}, - SeccompProfile: &corev1.SeccompProfile{Type: corev1.SeccompProfileTypeRuntimeDefault}, - } podSpec.Containers = append(podSpec.Containers, rbacProxyContainer) podSpec.Volumes = append(podSpec.Volumes, corev1.Volume{ Name: volumeName, diff --git a/openshift-knative-operator/pkg/monitoring/rbac_proxy_injection_test.go b/openshift-knative-operator/pkg/monitoring/rbac_proxy_injection_test.go index c768982940..eb088278c5 100644 --- a/openshift-knative-operator/pkg/monitoring/rbac_proxy_injection_test.go +++ b/openshift-knative-operator/pkg/monitoring/rbac_proxy_injection_test.go @@ -13,7 +13,6 @@ import ( "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/client-go/kubernetes/scheme" - "knative.dev/pkg/ptr" ) func TestInjectRbacProxyContainerToDeployments(t *testing.T) { @@ -126,13 +125,6 @@ func TestInjectRbacProxyContainerToDeployments(t *testing.T) { "--logtostderr=true", "--v=10", }, - SecurityContext: &corev1.SecurityContext{ - AllowPrivilegeEscalation: ptr.Bool(false), - ReadOnlyRootFilesystem: ptr.Bool(true), - RunAsNonRoot: ptr.Bool(true), - Capabilities: &corev1.Capabilities{Drop: []corev1.Capability{"ALL"}}, - SeccompProfile: &corev1.SeccompProfile{Type: corev1.SeccompProfileTypeRuntimeDefault}, - }, }}, }, },