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

Fixes #374 - Revocation command update and crl renew #375

Merged
merged 1 commit into from
May 23, 2020

Conversation

Rubueno
Copy link
Contributor

@Rubueno Rubueno commented Mar 13, 2020

An issue was raised informing that the revocation command is incorrect.
This was diagnosed to indeed be the case. As the $name variable in
context of revoke.pp does not evalute to server name but instead
client name. The exec for the crl renew was updated to clarify which server
it's done for and to prevent duplicate exec resource names.

Fixes #374

@Rubueno Rubueno force-pushed the revokefix branch 2 times, most recently from add3ec3 to 3bfa748 Compare March 14, 2020 17:00
@Rubueno
Copy link
Contributor Author

Rubueno commented May 18, 2020

@bastelfreak could you have a look at this?

@bastelfreak bastelfreak requested a review from Dan33l May 18, 2020 15:29
@bastelfreak
Copy link
Member

I'm not very familiar with openvpn. Any chance to add an acceptance test for this?

@Rubueno Rubueno force-pushed the revokefix branch 16 times, most recently from 8442a18 to b6920df Compare May 18, 2020 18:43
@Rubueno
Copy link
Contributor Author

Rubueno commented May 18, 2020

@bastelfreak An acceptance test is written and tests valid. I've cleaned up the way the revoked client file is managed. The test I wrote succeeds, but it's failing on another part which I didn't touch, which also seems to fail on a clean fork from master.

@bastelfreak
Copy link
Member

ah, this pulls in a broken version of facter and is fixed in https://github.com/voxpupuli/puppet-openvpn/pull/380/files - but that introduced another error. we will have a look and come back to you.

@bastelfreak
Copy link
Member

might be fixed with #381

@Rubueno
Copy link
Contributor Author

Rubueno commented May 19, 2020

I'll subscribe to #381, I'll rebase to see if it's fixed then

@bastelfreak
Copy link
Member

looks good, please rebase :)

@Rubueno
Copy link
Contributor Author

Rubueno commented May 20, 2020

@bastelfreak Unsure why it's failing so massively now. Would it help to rerun the CI jobs?

@bastelfreak
Copy link
Member

This was caused by the beaker 4.23.0 release: https://rubygems.org/gems/beaker
4.23.1 was released which fixes it. I restarted the jobs.

@bastelfreak bastelfreak added the bug Something isn't working label May 21, 2020
@Rubueno
Copy link
Contributor Author

Rubueno commented May 22, 2020

Cheers. Seems all is good but it's not reflected back here.

An issue was raised informing that the revocation command is incorrect.
This was diagnosed to indeed be the case. As the `$name` variable in
context of `revoke.pp` does not evalute to `server name` but instead
`client name`. The exec for the crl renew was updated to clarify which server
it's done for and to prevent duplicate `exec` resource names.

`catch_changes` in the acceptance test was taken out because a crl renew
is triggrered which is seen as a change.
@bastelfreak bastelfreak merged commit 286f58a into voxpupuli:master May 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Problem while revoking certificate
2 participants