Make some recently required bool parameters optional

[tvoran]

(Just putting this up as an option to address #631 if we wish.)

Makes some currently required bool parameters optional, but keeps the default of false, so that the value can effectively be set to false in k8s storage. The secret destination options were added in v0.5.0.

I was able to duplicate the upgrade error in #631 on OpenShift outside of OperatorHub with operator-sdk run bundle... and operator-sdk run bundle-upgrade..., which would fail when going from v0.4.3 to v0.5.x, but succeed when going from v0.4.3 to these changes.

Details for posterity:

$ oc new-project vault-secrets-operator

$ operator-sdk run bundle -n vault-secrets-operator registry.connect.redhat.com/hashicorp/vault-secrets-operator-bundle:0.4.3

# deploy a VaultStaticSecret setup
...

# upgrade the operator
$ operator-sdk run bundle-upgrade -n vault-secrets-operator registry.connect.redhat.com/hashicorp/vault-secrets-operator-bundle:0.5.1

# ^ this will eventually timeout, with an error like this in the status of the subscription:
$ kubectl get subscription -n vault-secrets-operator vault-secrets-operator-v0-4-3-sub -o yaml
...
    message: 'error validating existing CRs against new CRD''s schema for "vaultstaticsecrets.secrets.hashicorp.com":
      error validating custom resource against new schema for VaultStaticSecret tenant-1/static-demo:
      [].spec.destination.overwrite: Required value'
...

# Build a bundle from this branch
$ export PREVIOUS_VERSION=v0.5.1
$ export TEST_VERSION=0.5.2-test-olm-upgrade
$ make ci-build ci-docker-build-ubi VERSION=${TEST_VERSION?}
$ docker tag hashicorp/vault-secrets-operator:${TEST_VERSION?}-ubi <docker-repo>:${TEST_VERSION?}-ubi
$ docker push <docker-repo>:${TEST_VERSION?}-ubi

$ make bundle IMG=<docker-repo>:${TEST_VERSION?} IMG_UBI=<docker-repo>:${TEST_VERSION?}-ubi VERSION=${TEST_VERSION?} USE_IMAGE_DIGESTS=true
$ make bundle-build BUNDLE_IMG=<docker-repo>-bundle:${TEST_VERSION?}
$ docker push <docker-repo>-bundle:${TEST_VERSION?}

# start over from the beginning, but do the bundle-upgrade with this branch, and the upgrade passes
...
$ operator-sdk run bundle-upgrade -n vault-secrets-operator docker.io/<docker-repo>-bundle:0.5.2-test-olm-upgrade

Closes #631

[tomhjp]

I'm supportive of this approach 👍 it always looked a bit wrong to me that these parameters were marked as required

[benashz]

We should revert the change made to SkipTLSVerify, after that +1

[benashz]

I think this one we want to keep as-is. We made this change under #527 - to avoid an issue with Rancher.

[tvoran]

We can certainly do that, especially since this has been in place since v0.4.3.

FWIW in my testing this behaves the same with or without omitempty when there's a default set. If the parameter is set to true/false in the CRD spec it comes back on a get. And if the parameter is unspecified it comes back as false on a get. Which I think satisfies #527 and #278.

[benashz]

I'm supportive of this approach 👍 it always looked a bit wrong to me that these parameters were marked as required

@tomhjp - yeah its unfortunate that fields with defaults are not excluded from the set of required fields, or that the validators do not take defaults into account.

[tvoran]

Because of these I think we'll want to use PREVIOUS_VERSION=v0.4.3 with make bundle at release time:

make bundle VERSION=${LATEST_TAG//v} USE_IMAGE_DIGESTS=true PREVIOUS_VERSION=v0.4.3

So that the resulting build/bundle/manifests/vault-secrets-operator.clusterserviceversion.yaml comes out like this:

  skips:
    - vault-secrets-operator.v0.5.0
    - vault-secrets-operator.v0.5.1
  version: 0.5.2
  replaces: vault-secrets-operator.v0.4.3