Merge pull request #16438 from juanvallejo/jvallejo/prevent-oc-rollou…

…t-panic

Automatic merge from submit-queue (batch tested with PRs 16861, 16438).

Prevent `oc rollout` panic when resource given is not a dc

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1493071

Prevents nil pointer dereference by skipping command logic
when given resource is not of type *deployapi.DeploymentConfig.

**Before**
```
$ oc rollout cancel rc/my-rc-1
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x189 pc=0x3821780]
...
```

**After**
```
$ oc rollout cancel rc/my-rc-1
error: expected deployment configuration, got *api.ReplicationController
```

cc @openshift/cli-review

examples/examples_test.go

@@ -121,6 +121,7 @@ func TestExampleObjectSchemas(t *testing.T) {
 		"../test/integration/testdata": {
 			// TODO fix this test to  handle json and yaml
 			"project-request-template-with-quota": nil, // skip a yaml file
+			"test-replication-controller":         nil, // skip &api.ReplicationController
 			"test-deployment-config":              &deployapi.DeploymentConfig{},
 			"test-image":                          &imageapi.Image{},
 			"test-image-stream":                   &imageapi.ImageStream{},

pkg/oc/cli/cmd/rollout/cancel.go

@@ -111,7 +111,8 @@ func (o CancelOptions) Run() error {
 	for _, info := range o.Infos {
 		config, ok := info.Object.(*deployapi.DeploymentConfig)
 		if !ok {
-			allErrs = append(allErrs, kcmdutil.AddSourceToErr("cancelling", info.Source, fmt.Errorf("expected deployment configuration, got %T", info.Object)))
+			allErrs = append(allErrs, kcmdutil.AddSourceToErr("cancelling", info.Source, fmt.Errorf("expected deployment configuration, got %s", info.Mapping.Resource)))
+			continue
 		}
 		if config.Spec.Paused {
 			allErrs = append(allErrs, kcmdutil.AddSourceToErr("cancelling", info.Source, fmt.Errorf("unable to cancel paused deployment %s/%s", config.Namespace, config.Name)))

test/cmd/deployments.sh

@@ -96,6 +96,11 @@ os::cmd::try_until_success 'oc rollout pause dc/database'
 os::cmd::try_until_text "oc get dc/database --template='{{.spec.paused}}'" "true"
 os::cmd::try_until_success 'oc rollout resume dc/database'
 os::cmd::try_until_text "oc get dc/database --template='{{.spec.paused}}'" "<no value>"
+# create a replication controller and attempt to perform `oc rollout cancel` on it.
+# expect an error about the resource type, rather than a panic or a success.
+os::cmd::expect_success 'oc create -f test/integration/testdata/test-replication-controller.yaml'
+os::cmd::expect_failure_and_text 'oc rollout cancel rc/test-replication-controller' 'expected deployment configuration, got replicationcontrollers'
+
 echo "rollout: ok"
 os::test::junit::declare_suite_end
 

test/integration/testdata/test-replication-controller.yaml

@@ -0,0 +1,31 @@
+apiVersion: v1
+kind: ReplicationController
+metadata:
+  annotations:
+    openshift.io/deployment-config.latest-version: "1"
+    openshift.io/deployment-config.name: test-deployment
+    openshift.io/deployment.phase: Complete
+    optnshift.io/deployment.replicas: "1"
+  name: test-replication-controller
+spec:
+  replicas: 1
+  selector:
+    deployment: test-deployment
+    deploymentconfig: test-deployment
+  template:
+    metadata:
+      labels:
+        deployment: test-deployment
+        deploymentconfig: test-deployment
+    spec:
+      containers:
+      - image: openshift/origin-pod
+        imagePullPolicy: IfNotPresent
+        name: ruby-helloworld
+        ports:
+        - containerPort: 8080
+          protocol: TCP
+        resources: {}
+      dnsPolicy: ClusterFirst
+      restartPolicy: Always
+status: {}