Add cloud function secrets tests

[wiktorn]

Additional tests for #1703 to catch such errors in the future.

In this case, for Cloud Functions v2 test, if I only want to test secret_environment_variables and secret_volumes, I still need to specify all non-important attributes in inventory, e.g.:

values:
  module.cf-http.google_cloudfunctions2_function.function:
    name: test-cf-http
    service_config:
      - all_traffic_on_latest_revision: true
        available_cpu: '0.166'
        available_memory: 256M
        environment_variables: null
        ingress_settings: ALLOW_ALL
        max_instance_count: 1
        min_instance_count: 0
        secret_environment_variables:
          - key: VARIABLE_SECRET
            project_id: '1234567890'
            secret: var_secret
            version: latest
        secret_volumes:
          - mount_path: /app/secret
            project_id: '1234567890'
            secret: path_secret
            versions:
              - path: first
                version: '1'
              - path: second
                version: '2'
              - path: latest
                version: latest
        timeout_seconds: 180
        vpc_connector: null
        vpc_connector_egress_settings: null

Which is painful and requires unnecessary maintenance work in the future. Instead, test fixture was modified so no matter how nested the object is defined, only keys and values provided in the inventory are validated against the plan.

After the change, inventory contains only relevant info:

  module.cf-http.google_cloudfunctions2_function.function:
    name: test-cf-http
    service_config:
      - secret_environment_variables:
          - key: VARIABLE_SECRET
            project_id: '1234567890'
            secret: var_secret
            version: latest
        secret_volumes:
          - mount_path: /app/secret
            project_id: '1234567890'
            secret: path_secret
            versions:
              - path: first
                version: '1'
              - path: second
                version: '2'
              - path: latest
                version: latest

---

Checklist

I applicable, I acknowledge that I have:

• Read the contributing guide
• Ran terraform fmt on all modified files
• Regenerated the relevant README.md files using tools/tfdoc.py
• Made sure all relevant tests pass

[wiktorn]

Let me check here, if all tests will succeed.

[juliocc]

lgtm in principle.

Left a few comments regarding the new validate_plan_object function

[ludoo]

Plus one on the change, even though I don't agree with the use case: when you need an inventory just do not declare one and run the test, it spits out a fully made inventory for you (pytest -s required). You don't need to manually build it, and it's absolutely not painful.

The next person touching the CF module's secret example will override your nice compact inventories with generated ones containing all values. :)

[wiktorn]

The next person touching the CF module's secret example will override your nice compact inventories with generated ones containing all values. :)

This good question maybe to consider in CONTRIBUTING.md - shall the tests cover platform defaults / module (like Cloud Function sizing etc)? I really like the idea, that we are asserting the stuff that was provided in the example and ignore the rest, as those either should not be covered at all (platform defaults), or in specific example (module defaults).

I do run pytest -s and then strip the result (and use that occasion to check, if all that I expected is indeed in the inventory - in this case I was originally missing some stuff due to bug in example :-))

But maybe this is just not worth the hassle.

[ludoo]

The next person touching the CF module's secret example will override your nice compact inventories with generated ones containing all values. :)

This good question maybe to consider in CONTRIBUTING.md - shall the tests cover platform defaults / module (like Cloud Function sizing etc)? I really like the idea, that we are asserting the stuff that was provided in the example and ignore the rest, as those either should not be covered at all (platform defaults), or in specific example (module defaults).

I do run pytest -s and then strip the result (and use that occasion to check, if all that I expected is indeed in the inventory - in this case I was originally missing some stuff due to bug in example :-))

But maybe this is just not worth the hassle.

your approach makes sense but is too labour intensive, I would not adopt any practice that slows development down :)

[ludoo]

Let me expand a tiny bit on what I wrote above: while having concise inventory with only the values under actual test is a sensible and nice approach, I think that

• the value of an inventory is also in catching provider changes like new attributes, different defaults, etc.
• the example is what really counts not the test, the test is mainly to make sure our documentation examples are up to date and work

So focusing too narrowly on the inventory takes us away from the main goal, and adds friction.

[juliocc]

I do agree with this change and I've actually had it in mind for a while. These are a couple situations that would be solved by this:

• Changes in the provider adding new values to existing attributes. Today we have to upgrade the provider, run all tests and manually add the new values. As one example, this commit, only updates nested dictionaries to include new values between provider versions. Today we have to fully specify the new values simply because they're checked for equality here. With this change, tests would still pass because the new fields from the plan would simply be ignored.
• No need to fully specify nested fields with complex structures. This is basically what @wiktorn is trying to solve, explained in the description of this pr.

when you need an inventory just do not declare one and run the test, it spits out a fully made inventory for you (pytest -s required). You don't need to manually build it, and it's absolutely not painful.

There are situations where you want an inventory, but not a very verbose one. The contributing guide is explicit in saying that you should trim down the generated inventory and keep only what is relevant to the use case you're testing.

I would not adopt any practice that slows development down :)

This PR would actually achieve the opposite :)

Using pytest -s is a quick way to build the initial inventory by looking at the plan generated by terraform. This PR doesn't affect that process, you can still do pytest -s and everything will keep working with the added benefit that your inventory will survive provider upgrades and you can make it slimmer if you want to.

As a side note, we should probably provide a better tool/process to build inventories, but that's a different discussion.

[ludoo]

I do said I like this change. 😀

I don't think I will ever spend the time to trim down inventories, personally. YMMV of course

[juliocc]

I don't think I will ever spend the time to trim down inventories, personally. YMMV of course

I do. Mine and other people's too 🤫

[juliocc]

One more thing: can you move this comment to the function as a docstring and update its content accordingly?

[wiktorn]

One more thing: can you move this comment to the function as a docstring and update its content accordingly?

Moved