Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid dynamic property definition, or warn about it #25

Closed
clintoncwolfe opened this issue Jun 11, 2018 · 1 comment
Closed

Avoid dynamic property definition, or warn about it #25

clintoncwolfe opened this issue Jun 11, 2018 · 1 comment

Comments

@clintoncwolfe
Copy link
Contributor

It appears inspec-gcp is performing API fetches, obtaining a deep data structure, then performing iteration over it to dynamically define methods representing properties of the resources.

This may be considered harmful, and advised against. The AWS resources do not do this; and the Azure resources are being re-written to avoid it.

This is a policy decision, and as such the GCP project can of course go its own way.

Some reasons why the inspec-core and inspec-azure work has decided to explicitly define properties and matchers:

  • If the underlying API changes, a user that has not changed InSpec versions would see different (broken?) behavior despite not changing their installation. This breaks semver, and reduces trust in InSpec, which after all is a security product.
  • Explicit properties give us the chance to write explicit tests for the properties, which means we can catch changes in CI
  • Explicit properties means we can write docs that are tuned to our audience (no expectation they know the GCP API)
  • It's a false economy. The actual implementation is a small portion of the work; docs and testing take up ~80% of the work anyway.
  • You can rename, hide, or adapt legacy API layers

There are of course strong arguments in the other direction as well. It cuts velocity painfully. And there are half measures (for example, having a list of expected properties and iterating over those).

If the project does decide to stick with dynamic properties, consider adding a warning or something similar to the README to alert users that the InSpec resources may change unexpectedly.

@skpaterson
Copy link

Closing as no longer relevant given the ongoing transition to a Magic Modules implementation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants