476: change-rule to delete packageInstall/app before service account and RBAC

pkg/kapp/config/default.go

@@ -515,6 +515,18 @@ changeGroupBindings:
   # delay other resources with load balancer provisioning
   # - apiVersionKindMatcher: {kind: Service, apiVersion: v1}
 
+- name: change-groups.kapp.k14s.io/serviceaccount
+  resourceMatchers: &serviceAccountMatchers
+  - apiVersionKindMatcher: {kind: ServiceAccount, apiVersion: v1}
+
+- name: change-groups.kapp.k14s.io/kapp-controller-app
+  resourceMatchers:
+  - apiVersionKindMatcher: {kind: App, apiVersion: kappctrl.k14s.io/v1alpha1}
+
+- name: change-groups.kapp.k14s.io/kapp-controller-packageinstall
+  resourceMatchers:
+  - apiVersionKindMatcher: {kind: PackageInstall, apiVersion: packaging.carvel.dev/v1alpha1}
+
 changeRuleBindings:
 # Insert CRDs before all CRs
 - rules:
@@ -582,6 +594,21 @@ changeRuleBindings:
       - notMatcher:
           matcher: *disableDefaultChangeGroupAnnMatcher
 
+- rules:
+  - "upsert before upserting change-groups.kapp.k14s.io/kapp-controller-packageinstall"
+  - "upsert before upserting change-groups.kapp.k14s.io/kapp-controller-app"
+  - "delete after deleting change-groups.kapp.k14s.io/kapp-controller-packageinstall"
+  - "delete after deleting change-groups.kapp.k14s.io/kapp-controller-app"
+  ignoreIfCyclical: true
+  resourceMatchers:
+  - andMatcher:
+      matchers:
+      - notMatcher: {matcher: *disableDefaultChangeGroupAnnMatcher}
+      - anyMatcher:
+          matchers:
+          - anyMatcher: {matchers: *serviceAccountMatchers}
+          - anyMatcher: {matchers: *rbacMatchers}
+
 - rules:
   - "upsert after upserting change-groups.kapp.k14s.io/storage-class"
   ignoreIfCyclical: true

pkg/kapp/diffgraph/change_graph_test.go

@@ -590,6 +590,125 @@ roleRef:
 	require.Equal(t, expectedOutput, output)
 }
 
+func TestChangeGraphWithAppCR_RoleRoleBindingAndSA(t *testing.T) {
+	yaml := `
+apiVersion: v1
+kind: ServiceAccount
+metadata:
+  name: default-ns-sa
+---
+kind: Role
+apiVersion: rbac.authorization.k8s.io/v1
+metadata:
+  name: default-ns-role
+rules:
+- apiGroups: ["*"]
+  resources: ["*"]
+  verbs: ["*"]
+---
+kind: RoleBinding
+apiVersion: rbac.authorization.k8s.io/v1
+metadata:
+  name: default-ns-role-binding
+subjects:
+- kind: ServiceAccount
+  name: default-ns-sa
+roleRef:
+  apiGroup: rbac.authorization.k8s.io
+  kind: Role
+  name: default-ns-role
+---
+apiVersion: kappctrl.k14s.io/v1alpha1
+kind: App
+metadata:
+  name: simple-app-cr
+spec:
+  serviceAccountName: default-ns-sa
+  fetch:
+  - git: {}
+  template:
+  - ytt: {}
+  deploy:
+  - kapp: {}
+---
+apiVersion: packaging.carvel.dev/v1alpha1
+kind: PackageInstall
+metadata:
+  name: pkg-demo
+spec:
+  serviceAccountName: default-ns-sa
+  packageRef:
+    refName: simple-app.corp.com
+    versionSelection:
+      constraints: 1.0.0
+  values:
+  - secretRef:
+      name: pkg-demo-values
+---
+apiVersion: v1
+kind: Secret
+metadata:
+  name: pkg-demo-values
+stringData:
+  values.yml: |
+    ---
+    hello_msg: "to all my internet friends"
+`
+	_, conf, err := ctlconf.NewConfFromResourcesWithDefaults(nil)
+	require.NoErrorf(t, err, "Parsing conf defaults")
+
+	opts := buildGraphOpts{
+		resourcesBs:         yaml,
+		op:                  ctldgraph.ActualChangeOpUpsert,
+		changeGroupBindings: conf.ChangeGroupBindings(),
+		changeRuleBindings:  conf.ChangeRuleBindings(),
+	}
+
+	graph, err := buildChangeGraphWithOpts(opts, t)
+	require.NoErrorf(t, err, "Expected graph to build")
+
+	output := strings.TrimSpace(graph.PrintStr())
+	expectedOutput := strings.TrimSpace(`
+(upsert) serviceaccount/default-ns-sa (v1) cluster
+(upsert) role/default-ns-role (rbac.authorization.k8s.io/v1) cluster
+(upsert) rolebinding/default-ns-role-binding (rbac.authorization.k8s.io/v1) cluster
+  (upsert) role/default-ns-role (rbac.authorization.k8s.io/v1) cluster
+(upsert) app/simple-app-cr (kappctrl.k14s.io/v1alpha1) cluster
+  (upsert) serviceaccount/default-ns-sa (v1) cluster
+  (upsert) role/default-ns-role (rbac.authorization.k8s.io/v1) cluster
+  (upsert) rolebinding/default-ns-role-binding (rbac.authorization.k8s.io/v1) cluster
+    (upsert) role/default-ns-role (rbac.authorization.k8s.io/v1) cluster
+(upsert) packageinstall/pkg-demo (packaging.carvel.dev/v1alpha1) cluster
+  (upsert) serviceaccount/default-ns-sa (v1) cluster
+  (upsert) role/default-ns-role (rbac.authorization.k8s.io/v1) cluster
+  (upsert) rolebinding/default-ns-role-binding (rbac.authorization.k8s.io/v1) cluster
+    (upsert) role/default-ns-role (rbac.authorization.k8s.io/v1) cluster
+(upsert) secret/pkg-demo-values (v1) cluster
+`)
+	require.Equal(t, expectedOutput, output)
+
+	opts.op = ctldgraph.ActualChangeOpDelete
+	graph, err = buildChangeGraphWithOpts(opts, t)
+	require.NoErrorf(t, err, "Expected graph to build")
+
+	output = strings.TrimSpace(graph.PrintStr())
+	expectedOutput = strings.TrimSpace(`
+(delete) serviceaccount/default-ns-sa (v1) cluster
+  (delete) packageinstall/pkg-demo (packaging.carvel.dev/v1alpha1) cluster
+  (delete) app/simple-app-cr (kappctrl.k14s.io/v1alpha1) cluster
+(delete) role/default-ns-role (rbac.authorization.k8s.io/v1) cluster
+  (delete) packageinstall/pkg-demo (packaging.carvel.dev/v1alpha1) cluster
+  (delete) app/simple-app-cr (kappctrl.k14s.io/v1alpha1) cluster
+(delete) rolebinding/default-ns-role-binding (rbac.authorization.k8s.io/v1) cluster
+  (delete) packageinstall/pkg-demo (packaging.carvel.dev/v1alpha1) cluster
+  (delete) app/simple-app-cr (kappctrl.k14s.io/v1alpha1) cluster
+(delete) app/simple-app-cr (kappctrl.k14s.io/v1alpha1) cluster
+(delete) packageinstall/pkg-demo (packaging.carvel.dev/v1alpha1) cluster
+(delete) secret/pkg-demo-values (v1) cluster
+`)
+	require.Equal(t, expectedOutput, output)
+}
+
 func buildChangeGraph(resourcesBs string, op ctldgraph.ActualChangeOp, t *testing.T) (*ctldgraph.ChangeGraph, error) {
 	return buildChangeGraphWithOpts(buildGraphOpts{resourcesBs: resourcesBs, op: op}, t)
 }