[v1.0.x] Backports (#3899)

joelanford · asmacdo · JAORMX · web-flow · commit c48d05c22fdc · 2020-09-17T10:11:24.000-04:00
Co-authored-by: Austin Macdonald &lt;austin@redhat.com&gt;
Co-authored-by: Joe Lanford &lt;joe.lanford@gmail.com&gt;
Co-authored-by: Juan Osorio Robles &lt;jaosorior@redhat.com&gt;
Co-authored-by: Venkat Ramaraju &lt;37827279+VenkatRamaraju@users.noreply.github.com&gt;
Co-authored-by: Eric Stroczynski &lt;estroczy@redhat.com&gt;
Co-authored-by: mikeshng &lt;58747157+mikeshng@users.noreply.github.com&gt;
Co-authored-by: Camila Macedo &lt;camilamacedo86@gmail.com&gt;
diff --git a/changelog/fragments/bundle-validate-verbose.yaml b/changelog/fragments/bundle-validate-verbose.yaml
@@ -0,0 +1,4 @@
+entries:
+  - description: Fixed debug logging in the `bundle validate` subcommand of `operator-sdk`
+    kind: "bugfix"
+    breaking: false
diff --git a/changelog/fragments/csv-allnamespaces-default.yaml b/changelog/fragments/csv-allnamespaces-default.yaml
@@ -0,0 +1,5 @@
+entries:
+  - description: >
+      `generate <bundle|packagemanifests>` now generates a CSV base with only the `AllNamespaces` install mode
+      supported by default, since projects are cluster-scoped by default.
+    kind: bugfix
diff --git a/changelog/fragments/gen-bundle-namespaced.yaml b/changelog/fragments/gen-bundle-namespaced.yaml
@@ -0,0 +1,18 @@
+# entries is a list of entries to include in
+# release notes and/or the migration guide
+entries:
+  - description: >
+      When generating bundles and packagemanifests, remove `metadata.namespace` from
+      namespaced resources when writing them into the `manifests` directory to avoid
+      validation errors.
+
+    # kind is one of:
+    # - addition
+    # - change
+    # - deprecation
+    # - removal
+    # - bugfix
+    kind: bugfix
+
+    # Is this a breaking change?
+    breaking: false
diff --git a/changelog/fragments/helm-install-status-fix.yaml b/changelog/fragments/helm-install-status-fix.yaml
@@ -0,0 +1,4 @@
+entries:
+  - description: Fixed a bug that caused the Helm operator not to set the `InstallSuccessful` and `UpgradeSuccessful` status reasons when the status update fails during installation and upgrade.
+    kind: "bugfix"
+    breaking: false
diff --git a/changelog/fragments/helm-roles-permissions.yaml b/changelog/fragments/helm-roles-permissions.yaml
@@ -0,0 +1,16 @@
+# entries is a list of entries to include in
+# release notes and/or the migration guide
+entries:
+  - description: >
+      In Helm projects, fix operator permissions for Openshift deployments by adding a `<resource>/finalizers` rule in the operator's role.
+
+    # kind is one of:
+    # - addition
+    # - change
+    # - deprecation
+    # - removal
+    # - bugfix
+    kind: "bugfix"
+
+    # Is this a breaking change?
+    breaking: false
diff --git a/changelog/fragments/operatorgroup-conflict.yaml b/changelog/fragments/operatorgroup-conflict.yaml
@@ -0,0 +1,7 @@
+entries:
+  - description: >
+      Prevent `run packagemanifests` from creating an OperatorGroup if one already exists in a namespace,
+      and use that OperatorGroup if its target namespaces exactly match those passed in `--install-mode`.
+      See [#3681](https://github.com/operator-framework/operator-sdk/issues/3681).
+    kind: bugfix
+    breaking: false
diff --git a/changelog/fragments/rescue-reconciliation.yaml b/changelog/fragments/rescue-reconciliation.yaml
@@ -0,0 +1,9 @@
+entries:
+  - description: >
+      Stop reconciling tasks when the event raised is a rescue in Ansible-based Operators.
+      More info: [Bugzilla 1856714](https://bugzilla.redhat.com/show_bug.cgi?id=1856714)
+
+    kind: "bugfix"
+
+    breaking: false
+    pull_request_override: 3650
diff --git a/changelog/fragments/scorecard_scaffolded_version.yaml b/changelog/fragments/scorecard_scaffolded_version.yaml
@@ -0,0 +1,16 @@
+# entries is a list of entries to include in
+# release notes and/or the migration guide
+entries:
+  - description: >
+      When scaffolding scorecard configurations, use release versions instead of `latest` in image tags.
+
+    # kind is one of:
+    # - addition
+    # - change
+    # - deprecation
+    # - removal
+    # - bugfix
+    kind: "bugfix"
+
+    # Is this a breaking change?
+    breaking: false
diff --git a/internal/ansible/controller/reconcile.go b/internal/ansible/controller/reconcile.go
@@ -186,7 +186,7 @@ func (r *AnsibleOperatorReconciler) Reconcile(request reconcile.Request) (reconc
 				return reconcile.Result{}, err
 			}
 		}
-		if event.Event == eventapi.EventRunnerOnFailed && !event.IgnoreError() {
+		if event.Event == eventapi.EventRunnerOnFailed && !event.IgnoreError() && !event.Rescued() {
 			failureMessages = append(failureMessages, event.GetFailedPlaybookMessage())
 		}
 	}
diff --git a/internal/ansible/runner/eventapi/types.go b/internal/ansible/runner/eventapi/types.go
@@ -122,3 +122,15 @@ func (je JobEvent) IgnoreError() bool {
 	}
 	return false
 }
+
+// Rescued - Detects whether or not a task was rescued
+func (je JobEvent) Rescued() bool {
+	if rescued, contains := je.EventData["rescued"]; contains {
+		for _, v := range rescued.(map[string]interface{}) {
+			if int(v.(float64)) == 1 {
+				return true
+			}
+		}
+	}
+	return false
+}
diff --git a/internal/cmd/operator-sdk/bundle/validate.go b/internal/cmd/operator-sdk/bundle/validate.go
@@ -93,16 +93,11 @@ func makeValidateCmd() *cobra.Command {
 		Use:   "validate",
 		Short: "Validate an operator bundle",
 		RunE: func(cmd *cobra.Command, args []string) (err error) {
-			if viper.GetBool(flags.VerboseOpt) {
-				log.SetLevel(log.DebugLevel)
-			}
-
 			// Always print non-output logs to stderr as to not pollute actual command output.
 			// Note that it allows the JSON result be redirected to the Stdout. E.g
 			// if we run the command with `| jq . > result.json` the command will print just the logs
 			// and the file will have only the JSON result.
-			logger := log.NewEntry(internal.NewLoggerTo(os.Stderr))
-
+			logger := createLogger(viper.GetBool(flags.VerboseOpt))
 			if err = c.validate(args); err != nil {
 				return fmt.Errorf("invalid command args: %v", err)
 			}
@@ -126,6 +121,15 @@ func makeValidateCmd() *cobra.Command {
 	return cmd
 }
 
+// createLogger creates a new logrus Entry that is optionally verbose.
+func createLogger(verbose bool) *log.Entry {
+	logger := log.NewEntry(internal.NewLoggerTo(os.Stderr))
+	if verbose {
+		logger.Logger.SetLevel(log.DebugLevel)
+	}
+	return logger
+}
+
 // validate verifies the command args
 func (c bundleValidateCmd) validate(args []string) error {
 	if len(args) != 1 {
diff --git a/internal/cmd/operator-sdk/bundle/validate_test.go b/internal/cmd/operator-sdk/bundle/validate_test.go
@@ -18,6 +18,7 @@ import (
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
 	"github.com/operator-framework/operator-sdk/internal/cmd/operator-sdk/bundle/internal"
+	log "github.com/sirupsen/logrus"
 )
 
 var _ = Describe("Running a bundle validate command", func() {
@@ -38,6 +39,19 @@ var _ = Describe("Running a bundle validate command", func() {
 		})
 	})
 
+	Describe("Creating a logger", func() {
+		It("that is Info Level when not verbose", func() {
+			verbose := false
+			logger := createLogger(verbose)
+			Expect(logger.Logger.GetLevel()).To(Equal(log.InfoLevel))
+		})
+		It("that is Debug level if verbose", func() {
+			verbose := true
+			logger := createLogger(verbose)
+			Expect(logger.Logger.GetLevel()).To(Equal(log.DebugLevel))
+		})
+	})
+
 	Describe("validate", func() {
 		var cmd bundleValidateCmd
 		BeforeEach(func() {
diff --git a/internal/cmd/operator-sdk/generate/internal/genutil_suite_test.go b/internal/cmd/operator-sdk/generate/internal/genutil_suite_test.go
@@ -0,0 +1,27 @@
+// Copyright 2020 The Operator-SDK 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 genutil
+
+import (
+	"testing"
+
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+)
+
+func TestGenutil(t *testing.T) {
+	RegisterFailHandler(Fail)
+	RunSpecs(t, "Collector Suite")
+}
diff --git a/internal/cmd/operator-sdk/generate/internal/manifests.go b/internal/cmd/operator-sdk/generate/internal/manifests.go
@@ -41,5 +41,19 @@ func GetManifestObjects(c *collector.Manifests) (objs []controllerutil.Object) {
 	_, clusterRoleObjs := c.SplitCSVClusterPermissionsObjects()
 	objs = append(objs, clusterRoleObjs...)
 
+	removeNamespace(objs)
 	return objs
 }
+
+// removeNamespace removes the namespace field of resources intended to be inserted into
+// an OLM manifests directory.
+//
+// This is required to pass OLM validations which require that namespaced resources do
+// not include explicit namespace settings. OLM automatically installs namespaced
+// resources in the same namespace that the operator is installed in, which is determined
+// at runtime, not bundle/packagemanifests creation time.
+func removeNamespace(objs []controllerutil.Object) {
+	for _, obj := range objs {
+		obj.SetNamespace("")
+	}
+}
diff --git a/internal/cmd/operator-sdk/generate/internal/manifests_test.go b/internal/cmd/operator-sdk/generate/internal/manifests_test.go
@@ -0,0 +1,56 @@
+// Copyright 2020 The Operator-SDK 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 genutil
+
+import (
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+	corev1 "k8s.io/api/core/v1"
+	rbacv1 "k8s.io/api/rbac/v1"
+	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
+	apiextensionsv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
+	"github.com/operator-framework/operator-sdk/internal/generate/collector"
+)
+
+var _ = Describe("GetManifestObjects", func() {
+	It("should unset the namespace", func() {
+		m := collector.Manifests{
+			Roles: []rbacv1.Role{
+				{ObjectMeta: metav1.ObjectMeta{Namespace: "foo", Name: "foo"}},
+				{ObjectMeta: metav1.ObjectMeta{Namespace: "bar"}},
+			},
+			ClusterRoles: []rbacv1.ClusterRole{
+				{ObjectMeta: metav1.ObjectMeta{Namespace: "bar"}},
+			},
+			ServiceAccounts: []corev1.ServiceAccount{
+				{ObjectMeta: metav1.ObjectMeta{Namespace: "foo", Name: "foo"}},
+				{ObjectMeta: metav1.ObjectMeta{Namespace: "bar"}},
+			},
+			V1beta1CustomResourceDefinitions: []apiextensionsv1beta1.CustomResourceDefinition{
+				{ObjectMeta: metav1.ObjectMeta{Namespace: "bar"}},
+			},
+			V1CustomResourceDefinitions: []apiextensionsv1.CustomResourceDefinition{
+				{ObjectMeta: metav1.ObjectMeta{Namespace: "bar"}},
+			},
+		}
+		objs := GetManifestObjects(&m)
+		Expect(objs).To(HaveLen(len(m.Roles) + len(m.ClusterRoles) + len(m.ServiceAccounts) + len(m.V1CustomResourceDefinitions) + len(m.V1beta1CustomResourceDefinitions)))
+		for _, obj := range objs {
+			Expect(obj.GetNamespace()).To(BeEmpty())
+		}
+	})
+})
diff --git a/internal/generate/clusterserviceversion/bases/clusterserviceversion.go b/internal/generate/clusterserviceversion/bases/clusterserviceversion.go
@@ -161,8 +161,8 @@ func (b ClusterServiceVersion) makeNewBase() *v1alpha1.ClusterServiceVersion {
 			Keywords:    b.Keywords,
 			Icon:        b.Icon,
 			InstallModes: []v1alpha1.InstallMode{
-				{Type: v1alpha1.InstallModeTypeOwnNamespace, Supported: true},
-				{Type: v1alpha1.InstallModeTypeSingleNamespace, Supported: true},
+				{Type: v1alpha1.InstallModeTypeOwnNamespace, Supported: false},
+				{Type: v1alpha1.InstallModeTypeSingleNamespace, Supported: false},
 				{Type: v1alpha1.InstallModeTypeMultiNamespace, Supported: false},
 				{Type: v1alpha1.InstallModeTypeAllNamespaces, Supported: true},
 			},
diff --git a/internal/generate/testdata/clusterserviceversions/bases/memcached-operator.clusterserviceversion.yaml b/internal/generate/testdata/clusterserviceversions/bases/memcached-operator.clusterserviceversion.yaml
@@ -24,9 +24,9 @@ spec:
       deployments: null
     strategy: ""
   installModes:
-  - supported: true
+  - supported: false
     type: OwnNamespace
-  - supported: true
+  - supported: false
     type: SingleNamespace
   - supported: false
     type: MultiNamespace
diff --git a/internal/generate/testdata/clusterserviceversions/bases/with-ui-metadata.clusterserviceversion.yaml b/internal/generate/testdata/clusterserviceversions/bases/with-ui-metadata.clusterserviceversion.yaml
@@ -33,9 +33,9 @@ spec:
       deployments: null
     strategy: ""
   installModes:
-  - supported: true
+  - supported: false
     type: OwnNamespace
-  - supported: true
+  - supported: false
     type: SingleNamespace
   - supported: false
     type: MultiNamespace
diff --git a/internal/generate/testdata/clusterserviceversions/output/memcached-operator.clusterserviceversion.yaml b/internal/generate/testdata/clusterserviceversions/output/memcached-operator.clusterserviceversion.yaml
@@ -149,9 +149,9 @@ spec:
         serviceAccountName: default
     strategy: deployment
   installModes:
-  - supported: true
+  - supported: false
     type: OwnNamespace
-  - supported: true
+  - supported: false
     type: SingleNamespace
   - supported: false
     type: MultiNamespace
diff --git a/internal/helm/controller/reconcile.go b/internal/helm/controller/reconcile.go
@@ -316,6 +316,20 @@ func (r HelmOperatorReconciler) Reconcile(request reconcile.Request) (reconcile.
 	}
 
 	log.Info("Reconciled release")
+	reason := types.ReasonUpgradeSuccessful
+	if expectedRelease.Version == 1 {
+		reason = types.ReasonInstallSuccessful
+	}
+	message := ""
+	if expectedRelease.Info != nil {
+		message = expectedRelease.Info.Notes
+	}
+	status.SetCondition(types.HelmAppCondition{
+		Type:    types.ConditionDeployed,
+		Status:  types.StatusTrue,
+		Reason:  reason,
+		Message: message,
+	})
 	status.DeployedRelease = &types.HelmAppRelease{
 		Name:     expectedRelease.Name,
 		Manifest: expectedRelease.Manifest,
diff --git a/internal/olm/operator/operator_suite_test.go b/internal/olm/operator/operator_suite_test.go
@@ -0,0 +1,27 @@
+// Copyright 2020 The Operator-SDK 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 olm
+
+import (
+	"testing"
+
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+)
+
+func TestOperator(t *testing.T) {
+	RegisterFailHandler(Fail)
+	RunSpecs(t, "Operator Suite")
+}
diff --git a/internal/olm/operator/packagemanifests_manager.go b/internal/olm/operator/packagemanifests_manager.go
diff --git a/internal/olm/operator/tenancy.go b/internal/olm/operator/tenancy.go
diff --git a/internal/olm/operator/tenancy_test.go b/internal/olm/operator/tenancy_test.go
diff --git a/internal/plugins/ansible/v1/scaffolds/internal/templates/config/rbac/role.go b/internal/plugins/ansible/v1/scaffolds/internal/templates/config/rbac/role.go
diff --git a/internal/plugins/scorecard/init.go b/internal/plugins/scorecard/init.go
diff --git a/internal/scorecard/examples/custom-scorecard-tests/bundle/manifests/memcached-operator.clusterserviceversion.yaml b/internal/scorecard/examples/custom-scorecard-tests/bundle/manifests/memcached-operator.clusterserviceversion.yaml
diff --git a/internal/scorecard/testdata/bundle.tar.gz b/internal/scorecard/testdata/bundle.tar.gz
diff --git a/internal/scorecard/testdata/bundle/manifests/memcached-operator.clusterserviceversion.yaml b/internal/scorecard/testdata/bundle/manifests/memcached-operator.clusterserviceversion.yaml
diff --git a/website/content/en/docs/building-operators/golang/operator-scope.md b/website/content/en/docs/building-operators/golang/operator-scope.md

Original file line number	Diff line number	Diff line change
`@@ -186,7 +186,7 @@ func (r *AnsibleOperatorReconciler) Reconcile(request reconcile.Request) (reconc`
`186`	`186`	`return reconcile.Result{}, err`
`187`	`187`	`}`
`188`	`188`	`}`
`189`		`- if event.Event == eventapi.EventRunnerOnFailed && !event.IgnoreError() {`
	`189`	`+ if event.Event == eventapi.EventRunnerOnFailed && !event.IgnoreError() && !event.Rescued() {`
`190`	`190`	`failureMessages = append(failureMessages, event.GetFailedPlaybookMessage())`
`191`	`191`	`}`
`192`	`192`	`}`