Extra vars poc #203
Extra vars poc #203
Conversation
CHANGELOG.md
Outdated
| ## [1.2.3] - 2024-04-30 | ||
|
|
||
| ### Changed | ||
| - Enhancement for the lookup plugin to support the extra-vars |
There was a problem hiding this comment.
This should count as a backwards-compatible addition
| ## [1.2.3] - 2024-04-30 | |
| ### Changed | |
| - Enhancement for the lookup plugin to support the extra-vars | |
| ## [1.3.0] - 2024-04-30 | |
| ### Added | |
| - Enhancement for the lookup plugin to support the extra-vars |
There was a problem hiding this comment.
can you elaborate more on that John. should this be minor change or description must be updated?
There was a problem hiding this comment.
Bumping the version from 1.2.2 to 1.2.3 would imply that the only released additions were bug fixes, but this goes a bit beyond that. We're adding a new method of configuring the plugin, so as long as we maintain backwards compatibility, we should bump the version to 1.3.0 instead.
| authn_login = self.get_var_value("conjur_authn_login") | ||
| authn_api_key = self.get_var_value("conjur_authn_api_key") | ||
| cert_file = self.get_var_value("conjur_cert_file") | ||
| authn_token_file = self.get_var_value("conjur_authn_token_file") |
There was a problem hiding this comment.
I think we have some unexpected behavior here that breaks backwards compatibility.
Previously, we had a section in the DOCUMENTATION variable that described the conjur_authn_token_file variable, but it was being accessed exclusively as an environment variable, so the specified default value was not being applied. Now that we're accessing the value this way, the default value is being applied when we would expect it not to.
We either need to add some special handling for this variable to maintain backwards compatibility, or release this as part of a major version bump.
There was a problem hiding this comment.
In the past, the authn_token_file value was generated by either an environmental variable or the default path (/var/run/conjur/access-token).
The newest modifications allow for the authn_token_file value to be pulled from either the environmental variable, extra variables, or the default path (/var/run/conjur/access-token).
The behavior was the same as earlier, but we have updated only the ini name of authn_token_file.
There was a problem hiding this comment.
In the past, the authn_token_file value was generated by either an environmental variable or the default path (/var/run/conjur/access-token)
The last part of this isn't true - previously, the lookup plugin checked the CONJUR_AUTHN_TOKEN_FILE environment variable, and if didn't exist, no default value was applied, and instead the plugin retrieved a new authn token.
Check out this PR's Jenkins build, which is failing on the lookup plugin's end-to-end tests.
TASK [Retrieve Conjur variable] ************************************************
fatal: [localhost]: FAILED! => {"msg": "An unhandled exception occurred while templating
'{{lookup('conjur_variable', 'ansible/test-secret')}}'. Error was a
<class 'ansible.errors.AnsibleError'>, original message: An unhandled exception
occurred while running the lookup plugin 'conjur_variable'. Error was a
<class 'ansible.errors.AnsibleError'>, original message: Conjur authn token
file `/var/run/conjur/access-token` was not found on the host. Conjur authn token
file `/var/run/conjur/access-token` was not found on the host"}
The existing DOCUMENTATION variable section describing the authn_token_file parameter is inaccurate, but we aren't using self.get_option, so it isn't being enforced. Backwards compatibility should be maintained if we updated the new authn_token_file section to specify that it doesn't have a default value, and isn't required.
There was a problem hiding this comment.
To ensure backward compatibility for conjur_authn_token_file. We have removed the default key parameter in the conjur_authn_token_file documentation section and also added an extra check for conjur_authn_token_file.
if not variable_value and key != "conjur_authn_token_file":
raise AnsibleError("The value of the {0} variable is not set".format(key))
The token can either be selected from environmental/extra-vars variables or generated within the code.
There was a problem hiding this comment.
Thanks, @ramavenkata-loya!
CHANGELOG.md
Outdated
| ## [1.2.3] - 2024-04-30 | ||
|
|
||
| ### Changed | ||
| - Enhancement for the lookup plugin to support the extra-vars |
There was a problem hiding this comment.
Bumping the version from 1.2.2 to 1.2.3 would imply that the only released additions were bug fixes, but this goes a bit beyond that. We're adding a new method of configuring the plugin, so as long as we maintain backwards compatibility, we should bump the version to 1.3.0 instead.
john-odonnell
force-pushed
the
extra-vars-poc
branch
from
3fff6a1 to
261de1d
Compare
Desired Outcome
Please describe the desired outcome for this PR. Said another way, what was
the original request that resulted in these code changes? Feel free to copy
this information from the connected issue.
Implemented Changes
Describe how the desired outcome above has been achieved with this PR. In
particular, consider:
Connected Issue/Story
Resolves #[relevant GitHub issue(s), e.g. 76]
CyberArk internal issue ID: [insert issue ID]
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