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

@@ -33,6 +33,7 @@ type RecordedApp struct {
 	name                  string
 	nsName                string
 	isMigrated            bool
+	isDiffRun             bool
 	creationTimestamp     time.Time
 	appChangesUseAppLabel bool
 
@@ -48,7 +49,7 @@ func NewRecordedApp(name, nsName string, creationTimestamp time.Time, coreClient
 	identifiedResources ctlres.IdentifiedResources, appInDiffNsHintMsgFunc func(string) string, logger logger.Logger) *RecordedApp {
 
 	// Always trim suffix, even if user added it manually (to avoid double migration)
-	return &RecordedApp{strings.TrimSuffix(name, AppSuffix), nsName, false, creationTimestamp, false, coreClient, identifiedResources, appInDiffNsHintMsgFunc,
+	return &RecordedApp{strings.TrimSuffix(name, AppSuffix), nsName, false, false, creationTimestamp, false, coreClient, identifiedResources, appInDiffNsHintMsgFunc,
 		nil, logger.NewPrefixed("RecordedApp")}
 }
 
@@ -132,6 +133,10 @@ func (a *RecordedApp) UpdateUsedGVsAndGKs(gvs []schema.GroupVersion, gks []schem
 func (a *RecordedApp) CreateOrUpdate(prevAppName string, labels map[string]string, isDiffRun bool) (bool, error) {
 	defer a.logger.DebugFunc("CreateOrUpdate").Finish()
 
+	if isDiffRun {
+		a.isDiffRun = true
+	}
+
 	app, foundMigratedApp, err := a.find(a.fqName())
 	if err != nil {
 		return false, err
@@ -153,7 +158,7 @@ func (a *RecordedApp) CreateOrUpdate(prevAppName string, labels map[string]strin
 	}
 
 	if prevAppName == "" {
-		return true, a.create(labels, isDiffRun)
+		return true, a.create(labels)
 	}
 
 	app, foundMigratedPrevApp, err := a.find(prevAppName + AppSuffix)
@@ -176,7 +181,7 @@ func (a *RecordedApp) CreateOrUpdate(prevAppName string, labels map[string]strin
 		return false, a.renameConfigMap(app, a.name, a.nsName)
 	}
 
-	return true, a.create(labels, isDiffRun)
+	return true, a.create(labels)
 }
 
 func (a *RecordedApp) find(name string) (*corev1.ConfigMap, bool, error) {
@@ -190,7 +195,7 @@ func (a *RecordedApp) find(name string) (*corev1.ConfigMap, bool, error) {
 	return cm, true, nil
 }
 
-func (a *RecordedApp) create(labels map[string]string, isDiffRun bool) error {
+func (a *RecordedApp) create(labels map[string]string) error {
 	configMap := &corev1.ConfigMap{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      a.name,
@@ -225,12 +230,13 @@ 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)
+
+	if a.isDiffRun {
+		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
@@ -242,6 +248,10 @@ func (a *RecordedApp) updateApp(existingConfigMap *corev1.ConfigMap, labels map[
 		return err
 	}
 
+	if a.isDiffRun {
+		return nil
+	}
+
 	_, err = a.coreClient.CoreV1().ConfigMaps(a.nsName).Update(context.TODO(), existingConfigMap, metav1.UpdateOptions{})
 	if err != nil {
 		return fmt.Errorf("Updating app: %w", err)
@@ -266,6 +276,10 @@ func (a *RecordedApp) migrate(c *corev1.ConfigMap, labels map[string]string, new
 		return err
 	}
 
+	if a.isDiffRun {
+		return nil
+	}
+
 	_, err = a.coreClient.CoreV1().ConfigMaps(a.nsName).Update(context.TODO(), c, metav1.UpdateOptions{})
 	if err != nil {
 		return fmt.Errorf("Updating app: %w", err)
@@ -559,6 +573,10 @@ func (a *RecordedApp) update(doFunc func(*Meta)) error {
 		return err
 	}
 
+	if a.isDiffRun {
+		return nil
+	}
+
 	_, err = a.coreClient.CoreV1().ConfigMaps(a.nsName).Update(context.TODO(), change, metav1.UpdateOptions{})
 	if err != nil {
 		return fmt.Errorf("Updating app: %w", err)

test/e2e/diff_test.go

@@ -4,6 +4,7 @@
 package e2e
 
 import (
+	"fmt"
 	"strings"
 	"testing"
 
@@ -402,3 +403,85 @@ 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
+  namespace: __ns__
+---
+apiVersion: v1
+kind: Secret
+metadata:
+  name: scoped-sa
+  namespace: __ns__
+  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
+  namespace: __ns__
+rules:
+- apiGroups: [""]
+  resources: ["configmaps"]
+  verbs: ["get", "list", "watch"]
+---
+kind: RoleBinding
+apiVersion: rbac.authorization.k8s.io/v1
+metadata:
+  name: scoped-role-binding
+  namespace: __ns__
+subjects:
+- kind: ServiceAccount
+  name: scoped-sa
+  namespace: __ns__
+roleRef:
+  apiGroup: rbac.authorization.k8s.io
+  kind: Role
+  name: scoped-role
+  namespace: __ns__
+`
+
+	rbac = strings.ReplaceAll(rbac, "__ns__", env.Namespace)
+
+	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{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 using scoped context", 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)
+	})
+}