Allows option policy factory variable inputs #2956
Conversation
jvanderhoof
force-pushed
the
allow-option-factory-inputs
branch
from
7e8c22f to
a875f74
Compare
jvanderhoof
force-pushed
the
allow-option-factory-inputs
branch
from
a875f74 to
e9dc20a
Compare
![codeclimate[bot]](https://avatars.githubusercontent.com/in/9403?s=60&v=4){kind=small}
| @@ -12,9 +12,11 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. | |||
| ## [1.20.0] - 2023-08-16 | |||
|
|
|||
| ### Fixed | |||
| - Allow Factories with optional variables to save without error | |||
There was a problem hiding this comment.
Lists should be surrounded by blank lines
| (schema_variables.keys & factory_variables.keys).each do |schema_variable| | ||
| next unless factory_variables.key?(schema_variable) |
There was a problem hiding this comment.
nit: shouldn't have to check that schema_variable gets a positive response from factory_variables.key? because it's baked into the parent array (schema_variables.keys & factory_variables.keys).
There was a problem hiding this comment.
Good catch. This was left over after I realized finding the intersection of keys was much more effective.
There was a problem hiding this comment.
nit: These tests now ensure that omitted optional vars don't get in the way of a factory request - should we include a case to ensure an included optional var is picked up?
There was a problem hiding this comment.
Good call. This bug made me realize the edge cases not covered despite "100%" branch coverage 😞.
This commit fixes an issue where an error was raised if an optional factory value was not present in the request body.
The previous implementation was causing errors to fill up the logs because Runit couldn't find a key file (which had been deleted).
jvanderhoof
force-pushed
the
allow-option-factory-inputs
branch
from
e9dc20a to
9603717
Compare
| variable_id = @uri.encode_www_form_component("#{variable_path}/#{factory_variable}") | ||
| # Only set secrets defined in the policy and present in factory payload | ||
| (schema_variables.keys & factory_variables.keys).each do |schema_variable| | ||
|
|
There was a problem hiding this comment.
Extra empty line detected at block body beginning.
|
Code Climate has analyzed commit 9603717 and detected 3 issues on this pull request. Here's the issue category breakdown:
The test coverage on the diff in this pull request is 100.0% (50% is the threshold). This pull request will bring the total coverage in the repository to 88.4% (-0.1% change). View more on Code Climate. |
Desired Outcome
This PR fixes a small bug where optional variables which are not present in the request body cause an error to occur when attempting to save the factory variables.
Implemented Changes
Describe how the desired outcome above has been achieved with this PR. In
particular, consider:
Connected Issue/Story
N/A
Definition of Done
At least 1 todo must be completed in the sections below for the PR to be
merged.
Changelog
CHANGELOG update
Test coverage
changes, or
Documentation
READMEs) were updated in this PRBehavior
Security