THREESCALE-11396 Improved secret watched-by logic

[carlkyrillos]

Issue Link

JIRA: THREESCALE-11396

What

This PR improves on the existing secret watched-by logic in a few ways:

1. Previously the secret labels on the APIcast CR followed this pattern: secret.apicast.apps.3scale.net/{secret-UID: 'true', where the value was set to true regardless of whether the secret had the watched-by label. Now the value is set to false if the Secret doesn't have the watched-by label and true if the secret does have the watched-by label.
2. Previously the operator would annotate the apicast pod with apimanager.apps.3scale.net/{secret-name}: '{secret-resourceVersion}' whenever a secret was referenced in the APIcast CR. Now the apicast pod is only annotated if the referenced secret also has the watched-by label on it.
3. Previously, any and all changes to a watched secret would trigger the apicast deployment to rollout a new pod whose annotations contained the latest resourceVersion of the watched secret. Now the operator will only rollout new pods when there is a change to the secret's .data, i.e. changes to the watched secrets labels, annotations, etc. won't trigger a rollout even if though the secret's resourceVersion changed. This is accomplished through a new secret called hashed-secret-data that stores a hash of each watched secret's data using SHA256 encryption. Whenever the operator detects that a watched secret's resourceVersion has changed, it first takes a hash of the secret's current .data and if that matches the secret's entry in the hashed-secret-data secret, then the operator will ignore the change and prevent a rollout. If the .data has changed, the operator will update the apicast pod's annotations with the watched secret's latest resourceVersion which will trigger a rollout.

Verification Steps

1. Checkout this PR
2. Prepare the cluster for a local install:

   make download
   make install

3. Create a Namespace and the configuration Secret:

   
   export NAMESPACE=apicast-test
   oc new-project $NAMESPACE

cat << EOF | oc create -f - apiVersion: v1 kind: Secret metadata: name: apicast-config-secret namespace: $NAMESPACE type: Opaque stringData: config.json: | { "services": [ { "proxy": { "policy_chain": [ { "name": "apicast.policy.upstream", "configuration": { "rules": [{ "regex": "/", "url": "http://echo-api.3scale.net" }] } } ] } } ] } EOF

4. Create a custom environment Secret (without the watched-by label):
```bash
cat << EOF | oc create -f -
apiVersion: v1
kind: Secret
metadata:
  name: custom-env-1
  namespace: $NAMESPACE
type: Opaque
stringData:
  custom_env.lua: |
    local cjson = require('cjson')
    local PolicyChain = require('apicast.policy_chain')
    local policy_chain = context.policy_chain

    local logging_policy_config = cjson.decode([[
    {
      "enable_access_logs": false,
      "custom_logging": "\"{{request}}\" to service {{service.name}} and {{service.id}}"
    }
    ]])

    policy_chain:insert( PolicyChain.load_policy('logging', 'builtin', logging_policy_config), 1)

    return {
      policy_chain = policy_chain,
      port = { metrics = 9421 },
    }
EOF

5. Create an APIcast CR that references the custom-env-1 Secret:

   cat << EOF | oc create -f -
   apiVersion: apps.3scale.net/v1alpha1
   kind: APIcast
   metadata:
   name: example-apicast
   namespace: $NAMESPACE
   spec:
   embeddedConfigurationSecretRef:
   name: apicast-config-secret
   customEnvironments:
   - secretRef:
       name: custom-env-1
   EOF

6. Run the operator:

   make run

7. Wait for APIcast to be ready:

   oc get deployment apicast-example-apicast -oyaml | yq '.status'

8. Verify that the APIcast labels reference the two secrets' UIDs but the label values are false:

   oc get apicast example-apicast -oyaml | yq '.metadata.labels'

   The labels should look like this:

   secret.apicast.apps.3scale.net/86655a7f-c4bd-4d2a-b364-9fcdfe8eed96: "false"
   secret.apicast.apps.3scale.net/fdb8d92d-e446-4882-aa49-569329f5712d: "false"

9. Verify that the apicast pod annotations don't contain references to the secrets since neither of the secrets have the watched-by label:

   oc get pods -o yaml | yq '.items[0].metadata.annotations' | grep apicast.apps.3scale.net

10. Verify that the hashed-secret-data secret exists but is empty:

    oc get secret hashed-secret-data -oyaml

11. Add the watched-by label to the configuration secret:

    oc label secret apicast-config-secret apicast.apps.3scale.net/watched-by=apicast

12. Verify that the APIcast labels were updated and that one of secret labels has a value of true:

    oc get apicast example-apicast -oyaml | yq '.metadata.labels'

    The labels should now look like this:

    secret.apicast.apps.3scale.net/86655a7f-c4bd-4d2a-b364-9fcdfe8eed96: "false"
    secret.apicast.apps.3scale.net/fdb8d92d-e446-4882-aa49-569329f5712d: "true"

13. Once the new pod is ready, verify that it has an annotation referencing the config secret:

    oc get pods -o yaml | yq '.items[0].metadata.annotations' | grep apicast.apps.3scale.net

    There should be an annotation that looks like this:

    apicast.apps.3scale.net/gateway-configuration-secret-resource-version: "164701"

14. Verify that the hashed-secret-data secret has an entry for the config secret:

    oc get secret hashed-secret-data -oyaml

15. Edit the data in the config secret and verify a new pod is created:

    oc get pods

16. Once the pods have stabilized, add a label to the config secret and verify that no new pods are created even though the resourceVersion has changed:

    oc label secret apicast-config-secret dummy=label

[valerymo]

Hi @carlkyrillos I verified PR. Done very good, working as described in verification steps. Minor comments in code - up to you.

But I have Comments for design. Can you please look.

• Current approach is much stricter than the approach in 3scale PR https://github.com/3scale/3scale-operator/pull/861, where any change in the secret referenced in the CR (APImanager) by the label apimanager.apps.3scale.net/watched-by=apimanager triggers a pod re-creation.
• In the current solution of Operator - only a change in secret DATA triggers a Pod(s) re-creation (for secret that has label apicast.apps.3scale.net/watched-by=apicast).
  • all information used for the operator logic will have to be in secret Data ? (for secret with watched-by. label)
  • label and annotations in Secrets are useless ? (in secrets that have watched-by label)
• Can it conflict with the existing design/functionality ( that not in this PR). Was it analized, or maybe discussed with Eguzki?
• I think we need to open a separate task for regression testing.

Please correct me if I didn't understand something or was wrong. I added question to Design Task Thank you

[valerymo]

@carlkyrillos , seems I was wrong when said that Labels and annotations become useless in Secrets that received "watched-by" label/. Metadata still will be used in logics, just changes in it will not cause Pod(s) restarting. But still maybe need think more about this + Regression.

ok ...looked once more. It seems good. PR is very good. Code is clear. Validation notes are clear , precise, detailed, and easy for usage. Thanks.

[valerymo]

/lgtm