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

Add the changes of better missing data handling in ConjurCA to CHANGELOG #1806

Merged
merged 1 commit into from
Sep 7, 2020

Conversation

liavyona
Copy link
Contributor

@liavyona liavyona commented Sep 6, 2020

What does this PR do?

Add the changes of better missing data handling in ConjurCA to CHANGELOG

What ticket does this PR close?

Connected to #1315

@liavyona liavyona requested a review from a team as a code owner September 6, 2020 11:16
CHANGELOG.md Outdated
@@ -39,6 +39,9 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
- A new database migration step updates the fingerprints in slosilo. The FIPS compliance
update in `v1.8.0` caused the previous fingerprints to be invalid.
[cyberark/conjur#1584](https://github.com/cyberark/conjur/issues/1584)
- Verify that Kubernetes Authenticator variables exist and have value before retrieving them so that we raise a
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We now verify...

@liavyona liavyona force-pushed the add-conjurca-errors-changelog branch from ca69ba7 to fb338bd Compare September 7, 2020 06:19
CHANGELOG.md Outdated
Comment on lines 42 to 44
- Conjur now verifies that Kubernetes Authenticator variables exist and have value before retrieving them so that a
proper error will be raised if they aren't and enhance supportability.
[cyberark/conjur#1315](https://github.com/cyberark/conjur/issues/1315)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry for this but can you remove and enhance supportability?

@liavyona liavyona force-pushed the add-conjurca-errors-changelog branch from fb338bd to 5c2ef6d Compare September 7, 2020 07:07
@codeclimate
Copy link

codeclimate bot commented Sep 7, 2020

Code Climate has analyzed commit 5c2ef6d and detected 0 issues on this pull request.

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 86.8% (0.0% change).

View more on Code Climate.

@liavyona liavyona merged commit cc72ff6 into master Sep 7, 2020
@alexkalish
Copy link
Contributor

Thank you for making this change! Based on what you wrote, should we have a docs issue for this so that the error is documented?

@izgeri izgeri deleted the add-conjurca-errors-changelog branch September 8, 2020 17:24
@boazmichaely
Copy link

@orenbm @liavyona (CC @alexkalish )
I'm capturing this bug fix in the 11.7 release notes and I'm missing one piece of information. It is unclear to me which error is actually raised if the cert value is missing from conjur/authn-k8s/<service-id>/ca/cert because
Issue 1315 is ambiguous. It calls for
"a RequiredResourceMissing or RequiredSecretMissing error" to be raised

If I'm reading correctly what @liavyona wrote then the actual error raised is RequiredSecretMissing and this is what I captured in the release notes. Is my understanding 100% accurate?

Can you also please add a link here to where this is documented?
Thank you!

@orenbm
Copy link
Member

orenbm commented Sep 10, 2020

If the variable is not defined in the policy then a RequiredResourceMissing error will be raised. If the variable is defined but has no value then a RequiredSecretMissing error will be raised.

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

Successfully merging this pull request may close these issues.

4 participants