awslabs / seed-farmer

Seed-Farmer is an orchestration tool that works with AWS CodeSeeder and acts as an orchestration tool modeled after GitOps deployments. It has a CommandLine Interface based in Python, leverages modular code deployments defined by declarative manifests, and includes change detection and deployment optimization.
https://seed-farmer.readthedocs.io/en/latest/
Apache License 2.0
49 stars 15 forks source link

[BUG] Seedfarmer 4.0.4 doesn't cache information on AWS if it was provided as environment variables #718

Closed ktulhax closed 2 months ago

ktulhax commented 2 months ago

Describe the bug

The following error occurs in case if module that uses environment variable was deleted and enviroment variable was unset

seedfarmer.errors.seedfarmer_errors.InvalidManifestError: The environment variable (TEST_VARIABLE) is not available

To Reproduce Steps to reproduce the behavior:

  1. Have project with the only one module that uses environment variable TEST_VARIABLE
  2. export TEST_VARIABLE="some_value"
  3. Create inital deployment with seedfarmer apply
  4. unset TEST_VARIABLE and delete the module from yaml
  5. Do seedfarmer apply again
  6. See the error descibed above

Expected behavior It was expected that seedfarmer will remember the values of yaml parameters on AWS if they were provided via environment variables

dgraeber commented 2 months ago

@ktulhax What you have described is as-designed. As an SOP, SF persists the manifests as passed in so the deployment can be replicated. In your case, you deployed a module that has a reference to an env parameter (that manifest is persisted), then removed the module from the manifest and unset the env parameter. Now SF is trying to compare your new submission to what is persisted (to determine that the module needs to be destroyed) and cannot resolved the env parameter.

This is not a bug, but perhaps you can submit a feature request so we can evaluate the ramifications of persisting env parameters.

malachi-constant commented 2 months ago

In agreement here, not sure this behaviour should be changed unless there's some compelling use case that warrants it.