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

Feature/install certificate #48

Merged
merged 8 commits into from Jun 12, 2017

Conversation

Projects
None yet
3 participants
@aeden
Member

aeden commented May 25, 2017

First stab at a custom resource for installing issued certificate.

Includes basic unit and integration tests. I've left notes for things to improve as well based on feedback from folks at the ChefConf 2017 hackday.

@aeden aeden self-assigned this May 25, 2017

@aeden aeden requested review from martinisoft and onlyhavecans May 25, 2017

aeden added some commits May 25, 2017

@martinisoft

Pretty solid PR with some comments. Great work @aeden 👍

default_action :install
property :install_path, kind_of: String, name_property: true
property :certificate_common_name, kind_of: String, required: true

This comment has been minimized.

@martinisoft

martinisoft May 27, 2017

Member

Since we key on common name as part of the existence check, should that also be the name property?

This comment has been minimized.

@aeden

aeden May 30, 2017

Member

That's not the impression I got from @onlyhavecans - he indicated that since this is effectively a file resource that the file path should be the name property.

@@ -0,0 +1,8 @@
# TODO: verify with x509_certificate

This comment has been minimized.

@martinisoft

martinisoft May 27, 2017

Member

Are these for some other day in the future or this PR?

This comment has been minimized.

@aeden

aeden May 30, 2017

Member

Can be for now or later.

This comment has been minimized.

@onlyhavecans

onlyhavecans May 30, 2017

Contributor

If you know the command it's as easy as

describe command('my cool ssl check command') do
  its('stdout') { should match /totally right/ }
  its('exit_code') { should eq 0 }
end

This comment has been minimized.

@aeden

aeden May 30, 2017

Member

One of the folks who helped my at the hackday indicated there are full blown test helpers for public and private keys in inspec that remove the need for match. At least that's what I understood.

This comment has been minimized.

describe file('/etc/apache2/ssl/dnsimple.xyz.crt') do
its('content') { should match /-----BEGIN CERTIFICATE-----/ }
end
# TODO: verify with key_rsa

This comment has been minimized.

@martinisoft

martinisoft May 27, 2017

Member

Are these for some other day in the future or this PR?

This comment has been minimized.

@aeden

aeden May 30, 2017

Member

Can be for now or later.

This comment has been minimized.

@onlyhavecans

onlyhavecans May 30, 2017

Contributor

https://www.inspec.io/docs/reference/resources/key_rsa/ seems simple to impliment I think and could be put in here

Updated most of the requested changes. Unsure about the certificate_common_name

@aeden

This comment has been minimized.

Member

aeden commented Jun 7, 2017

@onlyhavecans @martinisoft I need a final review of this. If it's ready then I will merge it in. If not, please let me know what else needs to be changed.

@onlyhavecans

I love it! I'm very interested in trying this out and seeing growing to take more on

@martinisoft

This is awesome and I am super happy you contributed this. ❤️

@aeden aeden merged commit 45286b0 into master Jun 12, 2017

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
@aeden

This comment has been minimized.

Member

aeden commented Jun 12, 2017

Are there other steps beyond this to get this versioned and into the supermarket?

@onlyhavecans

This comment has been minimized.

Contributor

onlyhavecans commented Jun 13, 2017

@aeden No, once it is versioned and goes to supermarket you are 👍

@aeden aeden deleted the feature/install-certificate branch Jun 25, 2017

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