[Add] CEL rule for validating App and PackageInstall Spec

[varshaprasad96]

What this PR does / why we need it:

This PR:

• Adds KB marker to ensure that either spec.ServiceAccount or spec.Cluster is present in App CR.
• Bumps controller-tools to 0.10.0 to support CEL based validation marker. That is the latest version compatible with the k8s release the project is currently at.

Which issue(s) this PR fixes:

Fixes #1380

Does this PR introduce a user-facing change?

No. The validation already exists, this change performs the same action before the resource is created.

Additional Notes for your reviewer:

Review Checklist:

• Follows the developer guidelines
• Relevant tests are added or updated
• Relevant docs in this repo added or updated
• Relevant carvel.dev docs added or updated in a separate PR and there's
  a link to that PR
• Code is at least as readable and maintainable as it was before this
  change

Additional documentation e.g., Proposal, usage docs, etc.:

[prembhaskal]

one comment based on my first glance.
currently if both serviceAccount and cluster are specified in a PackageInstall CRD, the serviceAccount is read and used.
if none are specified, we get an error.

so validation in PR might break some existing configuration.
@praveenrewar @100mik Is current behaviour to choose serviceAccount over cluster config by design?

[praveenrewar]

@varshaprasad96 Could you run './hack/build.sh', './hack/gen.sh', and './hack/gen-apiserver.sh' and add the changes to the generated files as well.
With these changes,

• we will error out when serviceAccount and cluster both are not provided (✅ )
• we will error out when both serviceAccount and cluster are provided (Even though this is how it should be, like @prembhaskal mentioned this might be a breaking change for some folks, for example if they have an App CR where they have mentioned both serviceAccount and cluster, then they will start seeing this error when they upgrade to the newer version of kc.)

Is current behaviour to choose serviceAccount over cluster config by design?

I am not sure if this was intended for long term, but yes, this is the current expected behaviour (I think it's mentioned somewhere in the docs as well)

cc @neil-hickey @joaopapereira

[varshaprasad96]

@prembhaskal @praveenrewar I've modified the validation to just check the existence of atleast one of it. This should retain the current behaviour and fix the failing tests.

[praveenrewar]

Thank you so much for the PR @varshaprasad96. The changes look good to me, could you also add a quick e2e test here.

[prembhaskal]

Changes look good.

I saw praveen's comment late.

[praveenrewar]

(A few minor changes)

[prembhaskal]

all seems fine.
Well test is failing actually.

[prembhaskal]

I think test is failing because k8s version of minikube is set to 1.20 which does not support CEL validation.

kapp-controller/.github/workflows/test-gh.yml

Line 49 in b4886df

minikube start --driver=docker --kubernetes-version="1.20.15"

[praveenrewar]

We could bump k8s version in the tests, but I don't want us to indicate that we don't support any version before 1.26. Since the kind tests with the latest version are passing, I think we can add a check in this test to skip if k8s version is <1.26.x (as the validation won't work on older versions but there shouldn't be any other issue because of it).

[varshaprasad96]

Sorry for not being able to follow up due to other commitments. The recent changes should skip the test if the server version is less than 1.26.

[praveenrewar]

@varshaprasad96 The test seems to be failing while getting the server version (I haven't taken a closer look so not sure exactly which part).

[prembhaskal]

cel_validation_test.go:26: Error getting Kubernetes version: unable to load in-cluster configuration, KUBERNETES_SERVICE_HOST and KUBERNETES_SERVICE_PORT must be defined

failing because the test code is not running on the kind cluster.