Respect service type in environment and pipeline variable patching, add dry run

[tobias-richter]

I have detected a bug in the patching of the environment variables.
In the current implementation the check if an environment variable should be removed takes only the name into account but not the actual service type the variable is applied to.

Example

When we first apply the following environment variable:

- name: VARIABLE
  value: author variable
  type: string
  service: author

When removing the service and applying the environment variables with

- name: VARIABLE
  value: author variable
  type: string

This led to the state that two environment variables were present in the environment and the old author environment variable was not cleaned up.

I have now changed the implementation and we are now also using the applied service during envaluation what environment variables need to be updated / deleted.

Since the pipeline variables looked like they will also get additional service / steps soon (currently only build is available) I have also adjusted the logic for the pipeline variable update.

Since I have introduced a enum for possible environment variable services and due to the new added service support for pipeline variables I had to separate the environment variables from the pipeline variables.

I have now also added an optional --dry-run flag which allows a preview of what the environment / pipeline variable step will do.

[github-actions]

Please format your code using rustfmt: cargo fmt

Removing outdated review.

[BerSomBen]

Hi @tobias-richter,
I prepared a local setup to review this changes.

First I used the latest pippo version, to verify the issue described.
I added a Variable as described to a) a pipeline, b) an environment.
Then I modified the pipeline.yml accordingly and removed the service: author. Correct would be, to remove this Entry, and readd it with default Service. After the next pippo "pipeline variables set" The Service must be build and no longer author
But its not. So: I can confirm issue.

[BerSomBen]

Hi @tobias-richter,

when I use my unchanged working pippo.json that I used to confirm this issue on main, I ran into a "thread 'main' panicked"

     Running `target/debug/pippo -c pippo.json -p *** -e *** -i *** pipeline vars list`
thread 'main' panicked at src/errors.rs:46:84:
called `Result::unwrap()` on an `Err` value: Error("missing field `status`", line: 1, column: 573)
note: run with `RUST_BACKTRACE=1` environment variable to display a backt

[tobias-richter]

Hi @tobias-richter,

when I use my unchanged working pippo.json that I used to confirm this issue on main, I ran into a "thread 'main' panicked"

     Running `target/debug/pippo -c pippo.json -p *** -e *** -i *** pipeline vars list`
thread 'main' panicked at src/errors.rs:46:84:
called `Result::unwrap()` on an `Err` value: Error("missing field `status`", line: 1, column: 573)
note: run with `RUST_BACKTRACE=1` environment variable to display a backt

[tobias-richter]

Sorry, @BerSomBen accidentially closed the PR. I will get in touch with you. I also noticed that kind of issue in one of my environments when the environment variables where wrongly set.

[tobias-richter]

@BerSomBen the error message is kind of confusing because what is actually the case is that you have a pipeline variable configured for "author" service which is not correct!
When parsing the response it is then failing with your error message:

      {
        "name": "VARIABLE",
        "value": "author variable",
        "type": "string",
        "service": "author",
        "status": "ready"
      },

I have assigned #38 to you in order to add more resilience in that case. But with that solution every invalid value would also get an "invalid" value when serializing or deserializing.

[BerSomBen]

As discussed, I reviewed the second PR and added tiny improvements to your branch. Please have a look and feel free to merge towards this branch.
When its merged, I'll update the review here

[github-actions]

Please format your code using rustfmt: cargo fmt

Removing outdated review.

[BerSomBen]

hi @tobias-richter ,

i reviewed and tested the changes and I'm fine with it!

If there is an invalid variable in the current CM Environment or Pipeline Variables, the next vars set run will lead to an Error that requires manual interaction:

❌ ERROR, invalid service type detected for variable 'VARIABLE: "invalid"'

[BerSomBen]

ok