Skip API operations for diff run

Diff run should be usable when the authorised RBAC role doesn't have
write permissions for resources in the cluster.

Diff run on app creation used the dry run feature of the Kubernetes
API, to avoid persisting kapp ConfigMap changes. However this still
checks RBAC permissions, and errored if the role did not have write
permissions for ConfigMaps.

The kapp ConfigMap was also being updated when doing a diff run for an
existing app.

This skips the API operations for updating the ConfigMap when doing a
diff run.

Fixes #791.

Signed-off-by: Anshul Sirur <[email protected]>

pkg/kapp/app/recorded_app.go

@@ -138,6 +138,9 @@ func (a *RecordedApp) CreateOrUpdate(prevAppName string, labels map[string]strin
 	}
 	if foundMigratedApp {
 		a.isMigrated = true
+		if isDiffRun {
+			return false, nil
+		}
 		return false, a.updateApp(app, labels)
 	}
 
@@ -146,6 +149,9 @@ func (a *RecordedApp) CreateOrUpdate(prevAppName string, labels map[string]strin
 		return false, err
 	}
 	if foundNonMigratedApp {
+		if isDiffRun {
+			return false, nil
+		}
 		if a.isMigrationEnabled() {
 			return false, a.migrate(app, labels, a.fqName())
 		}
@@ -162,6 +168,9 @@ func (a *RecordedApp) CreateOrUpdate(prevAppName string, labels map[string]strin
 	}
 	if foundMigratedPrevApp {
 		a.isMigrated = true
+		if isDiffRun {
+			return false, nil
+		}
 		return false, a.renameConfigMap(app, a.fqName(), a.nsName)
 	}
 
@@ -170,6 +179,9 @@ func (a *RecordedApp) CreateOrUpdate(prevAppName string, labels map[string]strin
 		return false, err
 	}
 	if foundNonMigratedPrevApp {
+		if isDiffRun {
+			return false, nil
+		}
 		if a.isMigrationEnabled() {
 			return false, a.migrate(app, labels, a.fqName())
 		}
@@ -225,12 +237,12 @@ func (a *RecordedApp) create(labels map[string]string, isDiffRun bool) error {
 		return err
 	}
 
-	createOpts := metav1.CreateOptions{}
 	if isDiffRun {
-		createOpts.DryRun = []string{metav1.DryRunAll}
+		a.setMeta(*configMap)
+		return nil
 	}
-	app, err := a.coreClient.CoreV1().ConfigMaps(a.nsName).Create(context.TODO(), configMap, createOpts)
 
+	app, err := a.coreClient.CoreV1().ConfigMaps(a.nsName).Create(context.TODO(), configMap, metav1.CreateOptions{})
 	a.setMeta(*app)
 
 	return err

test/e2e/diff_test.go

@@ -4,6 +4,7 @@
 package e2e
 
 import (
+	"fmt"
 	"strings"
 	"testing"
 
@@ -402,3 +403,99 @@ metadata:
 
 	NewMissingClusterResource(t, "configmap", name, env.Namespace, kubectl)
 }
+
+func TestDiffRunReadOnlyContext(t *testing.T) {
+	env := BuildEnv(t)
+	logger := Logger{}
+	kapp := Kapp{t, env.Namespace, env.KappBinaryPath, Logger{}}
+	kubectl := Kubectl{t, env.Namespace, Logger{}}
+
+	rbac := `
+---
+apiVersion: v1
+kind: ServiceAccount
+metadata:
+  name: scoped-sa
+---
+apiVersion: v1
+kind: Secret
+metadata:
+  name: scoped-sa
+  annotations:
+    kubernetes.io/service-account.name: scoped-sa
+type: kubernetes.io/service-account-token
+---
+kind: Role
+apiVersion: rbac.authorization.k8s.io/v1
+metadata:
+  name: scoped-role
+rules:
+- apiGroups: [""]
+  resources: ["configmaps"]
+  verbs: ["get", "list", "watch"]
+---
+kind: RoleBinding
+apiVersion: rbac.authorization.k8s.io/v1
+metadata:
+  name: scoped-role-binding
+subjects:
+- kind: ServiceAccount
+  name: scoped-sa
+roleRef:
+  apiGroup: rbac.authorization.k8s.io
+  kind: Role
+  name: scoped-role
+`
+
+	rbacName := "test-e2e-rbac-app"
+	scopedContext := "scoped-context"
+	scopedUser := "scoped-user"
+	appName := "diff-run-read-only"
+
+	cleanUp := func() {
+		kapp.Run([]string{"delete", "-a", rbacName})
+		kapp.Run([]string{"delete", "-a", appName})
+	}
+	cleanUp()
+	defer cleanUp()
+
+	kapp.RunWithOpts([]string{"deploy", "-a", rbacName, "-f", "-"}, RunOpts{IntoNs: true, StdinReader: strings.NewReader(rbac)})
+	cleanUpContext := ScopedContext(t, kubectl, "scoped-sa", scopedContext, scopedUser)
+	defer cleanUpContext()
+
+	yaml1 := `
+apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: config
+`
+
+	logger.Section("diff-run app create using read-only role", func() {
+		_, _ = kapp.RunWithOpts([]string{"deploy", "-f", "-", "-a", appName, "--diff-run", fmt.Sprintf("--kubeconfig-context=%s", scopedContext)},
+			RunOpts{IntoNs: true, AllowError: false, StdinReader: strings.NewReader(yaml1)})
+
+		NewMissingClusterResource(t, "configmap", appName, env.Namespace, kubectl)
+	})
+
+	yaml2 := `
+apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: config
+  annotations:
+    foo: bar
+`
+
+	logger.Section("diff-run app update using read-only role", func() {
+		_, _ = kapp.RunWithOpts([]string{"deploy", "-f", "-", "-a", appName},
+			RunOpts{IntoNs: true, AllowError: false, StdinReader: strings.NewReader(yaml1)})
+
+		NewPresentClusterResource("configmap", appName, env.Namespace, kubectl)
+
+		_, _ = kapp.RunWithOpts([]string{"deploy", "-f", "-", "-a", appName, "--diff-run", fmt.Sprintf("--kubeconfig-context=%s", scopedContext)},
+			RunOpts{IntoNs: true, AllowError: false, StdinReader: strings.NewReader(yaml2)})
+
+		c := NewPresentClusterResource("configmap", appName, env.Namespace, kubectl)
+		require.NotContains(t, c.res.Annotations(), "foo", "Expected configmap to not have foo annotation")
+	})
+}