feat(reconcile): added a flag to skip validation of the namespace

[craicoverflow]

Motivation

https://issues.jboss.org/browse/AEROGEAR-9170

What

Added an option to skip checking if the operator namespace is valid during the Reconcile loop.

Why

The Reconcile function recently to check if the namespace provided in a CR is valid.

To do this, the Operator SDK reads the file at /var/run/secrets/kubernetes.io/serviceaccount/namespace to see if the CR namespace matches the one at this location.

This is fine when the operator is live, but /var/run/secrets/kubernetes.io/serviceaccount/namespace would not exist locally where unit tests are running in isolation, so this check will always fail.

This flag can be passed in a CR to skip this validation. It is a workaround and will be removed in the future when the Operator SDK is updated. See operator-framework/operator-sdk#1388

How

Added a new property to each CR spec, and if this is set to true then the namespace validation is skipped.

Verification Steps

Add the steps required to check this change. Following an example.

1. Go to file deploy/crds/mobile-security-service_v1alpha1_mobilesecurityservice_cr.yaml.
2. Add a new property to spec:

spec:
  skipNamespaceValidation: true

3. Go to pkg/controller/mobilesecurityservice/controller.go.
4. Add a breakpoint to line 156.
5. Go to pkg/controller/mobilesecurityservice/controller_test.go.
6. Debug the test TestReconcileMobileSecurityService_Reconcile.
7. The breakpoint in pkg/controller/mobilesecurityservice/controller.go should not be hit when skipNamespaceValidation is true.
8. Repeat 1-7 for the two other controllers.

Checklist:

• Code has been tested locally by PR requester
• Changes have been successfully verified by another team member

Progress

• Finished task
• TODO

[damienomurchu]

Changes look good 👍 I haven't verified them though.

[davidffrench]

Changes look good to me as well. Could you split the code specific to the bypass in a separate commit, this will make it easier to revert later on using just git.

[camilamacedo86]

Should we merge the test here? I am fine with that since we are in the initial dev phase. Just to be sure.

[craicoverflow]

I plan to remove this before merge as it is incomplete. It is just to allow you to verify this PR more easily.

[craicoverflow]

Changes look good to me as well. Could you split the code specific to the bypass in a separate commit, this will make it easier to revert later on using just git.

Changes look good to me as well. Could you split the code specific to the bypass in a separate commit, this will make it easier to revert later on using just git.

@davidffrench My editor automatically corrects formatting on save so I didn't really notice! Fixed now.

[camilamacedo86]

The code changes show fine for me. I'd just suggest for all places where you are passing the bool you add a comment with //FIXME: + small explanation over why it is used.

It also passed in the CI.
#!/bin/bash -eo pipefail
make test

Running tests:
ok  	github.com/aerogear/mobile-security-service-operator/pkg/controller/mobilesecurityservice	0.040s	coverage: 36.8% of statements

Great job 🥇