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

InSpec resources extend gcp_backend resource #930

Merged
merged 17 commits into from
Nov 26, 2018

Conversation

slevenick
Copy link
Contributor

InSpec resources inherit from base GcpResource class to handle auth/api requests. Single new resource generation for SSL policy


[all]

[terraform]

[terraform-beta]

[ansible]

[inspec]

Inherit from base GcpResource, SSL policy generation

@modular-magician
Copy link
Collaborator

I am a robot that works on MagicModules PRs!

I built this PR into one or more PRs on other repositories, and when those are closed, this PR will also be merged and closed.
depends: hashicorp/terraform-provider-google#2496
depends: modular-magician/inspec-gcp#33

@modular-magician
Copy link
Collaborator

I am (still) a robot that works on MagicModules PRs!

I just wanted to let you know that your changes (as of commit d36ef41) have been included in your existing downstream PRs.

@@ -24,11 +24,6 @@ apt-get update && apt-get install google-cloud-sdk -y

Copy link
Contributor

Choose a reason for hiding this comment

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

A lot of this file isn't quite right - downloading terraform, for instance, belongs in the container build process. So does installing unzip, so does installing the google cloud sdk, etc etc.

@@ -75,7 +64,7 @@ for i in {1..30}
do
# Cleanup cassettes folder each time, we don't want to use a recorded cassette if it records an unauthorized response
rm -r inspec-cassettes
inspec exec inspec-mm --attrs=attributes/attributes.yaml -t gcp2://
inspec exec verify-mm --attrs=attributes/attributes.yaml -t gcp:// --no-distinct-exit
if [ "$?" -eq "0" ]; then
Copy link
Contributor

Choose a reason for hiding this comment

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

The best way to do this is just

if inspec exec verify-mm .... ; then

The current way has an implicit dependency between the two lines which later editing might break.

@@ -25,6 +24,8 @@ if [ -z "$INSPEC_COMMIT_MSG" ]; then
fi

pushd "build/inspec"
# We are not ready to overwrite changelog yet.
git checkout CHANGELOG.md
Copy link
Contributor

Choose a reason for hiding this comment

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

best to specify where to check it out from, like git checkout HEAD -- CHANGELOG.md. This seems silly but you'd be surprised what can happen with git - in this case, if someone ever pushes a branch called CHANGELOG.md, you'd wind up in an absurd (and confused) state.

@nat-henderson
Copy link
Contributor

Could you revise the name of the PR, also? Or maybe just add a comment explaining it - I don't quite understand what it means right now.

@slevenick slevenick changed the title Inspec inherit gcp super InSpec resources extend gcp_backend resource Nov 21, 2018
@modular-magician
Copy link
Collaborator

I am (still) a robot that works on MagicModules PRs!

I just wanted to let you know that your changes (as of commit bf3f9c7) have been included in your existing downstream PRs.

@modular-magician
Copy link
Collaborator

I am (still) a robot that works on MagicModules PRs!

I just wanted to let you know that your changes (as of commit 45497d4) have been included in your existing downstream PRs.

Copy link
Contributor

@nat-henderson nat-henderson left a comment

Choose a reason for hiding this comment

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

This looks good to me!

slevenick and others added 2 commits November 26, 2018 22:17
@modular-magician modular-magician merged commit c0a3439 into master Nov 26, 2018
@slevenick slevenick deleted the inspec-inherit-gcp-super branch April 15, 2019 19:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants