New postgres_hba_conf resource #1964

aaronlippold · 2017-06-25T19:56:27Z

This is a resource for parsing and processing the pg_ident.conf file for a postgres database.

Signed-off-by: Rony Xavier <[email protected]>

… into al/pg_hba_conf

Signed-off-by: Aaron Lippold <[email protected]>

added test and doc files Signed-off-by: Rony Xavier <[email protected]>

updated test files and docs Signed-off-by: Rony Xavier <[email protected]>

Signed-off-by: Rony Xavier <[email protected]>

aaronlippold · 2017-06-25T20:35:38Z

@arlimus @chris-rock Appveyor seems to be running tests on windows targets even though I have a return skip_resource 'yada yada - no ones uses windows anyway' if inspec.os.windows? in place.

aaronlippold · 2017-06-26T03:42:20Z

See #1965 for an example of the issue we were having

adamleff

@aaronlippold thanks for this PR as well.

A lot of my feedback in #1963 applies to this resource so I'd like to ask you to apply the same feedback to this PR before I do a full review. Thank you!

adamleff · 2017-06-26T16:54:53Z

lib/resources/pg_hba_conf.rb

+
+    # @todo add checks to ensure that we have data in our file
+    def initialize(hba_conf_path = nil)
+      if inspec.os.linux?


This is better written as:

return skip_resource "blah blah" unless inspec.os.linux? # rest of method here

This will avoid unnecessary indenting.

Thank you Adam. I have made all the corrections to both the postgres PRs.
Please review them when you get a chance.
Thank you again.

Hi @rx294 I haven't seen an update to this PR but you mentioned you made corrections to both. Just wanted you to be aware in case you forgot to push up your changes. Thanks! I also left another change request on #1963 to help clean that up to get it ready. You'll likely have to make a similar change to this one in order to mock/stub the calls to the postgres resource.

Hey @adamleff , I am sorry about that, this push contains all the corrections and the test code fix.
Please take a look when you get chance.
Thank you

adamleff · 2017-06-26T16:55:26Z

lib/resources/pg_hba_conf.rb

+    # @todo add checks to ensure that we have data in our file
+    def initialize(hba_conf_path = nil)
+      if inspec.os.linux?
+        @conf_dir = inspec.postgres.conf_dir


Why do we need this as an instance variable, and why expose it to the user via attr_reader?

I guess we don't have to. Just habit really

adamleff · 2017-06-26T16:56:25Z

www/Gemfile.lock

@@ -1,7 +1,7 @@
 PATH


Your change should not include any www-related changes, please. Please revert this file in your next commit.

how do we do that?

adamleff · 2017-06-26T16:57:32Z

lib/inspec/resource.rb

@@ -122,6 +122,7 @@ def self.validate_resource_dsl_version!(version)
 require 'resources/packages'
 require 'resources/parse_config'
 require 'resources/passwd'
+require 'resources/pg_hba_conf'


Just like I mentioned in #1963, I'd like to suggest this resource be named postgres_hba_conf to keep all PostgreSQL-related resourced prefixed the same. While I know the file is named pg_hba.conf in practice, it's more important for the InSpec UX to be consistent.

Ok. I agree that works.

Signed-off-by: Rony Xavier <[email protected]>

'cmp' matcher instead of 'eq' Signed-off-by: Rony Xavier <[email protected]>

chris-rock

@rx294 @aaronlippold this is a great addition. I added some comments. Once resolved, we're ready for merge.

chris-rock · 2017-07-03T06:20:08Z

docs/resources/postgres_hba_conf.md.erb

+
+An `postgres_hba_conf` InSpec audit resource block declares client authentication data that should be tested:
+
+    describe postgres_hba_conf.where { pg_username == 'filter_value' } do


Should we use pg_username here? Shouldn't we use the more generic version here as you use in code?

describe postgres_hba_conf.where { type == 'local' } do its('auth_method') { should eq ['peer'] } end

chris-rock · 2017-07-03T06:23:09Z

lib/resources/postgres_hba_conf.rb

+      if raw_conf.empty? && !file.empty?
+        return skip_resource("Can't read the contents of \"#{@conf_file}\"")
+      end
+      @content = clean_conf_file(@conf_file)


Was there a specific reason why we haven't used simpleconfig https://github.com/chef/inspec/blob/master/lib/utils/simpleconfig.rb here?

This is not a blocker for merge, but I would like us to re-use as much as possible.

chris-rock · 2017-07-03T06:23:32Z

lib/resources/postgres_hba_conf.rb

@@ -0,0 +1,95 @@
+# encoding: utf-8
+# copyright: 2017


Lets remove the copyright header here, since everything is Apache licensed

…ater. Signed-off-by: Aaron Lippold <[email protected]>

chris-rock

Awesome @rx294 @aaronlippold Thank you for your great work

rx294 and others added 13 commits June 23, 2017 01:59

Created pg_hba_conf resource

bd192cc

Signed-off-by: Rony Xavier <[email protected]>

Created pg_hba_conf resource

91b28c8

Signed-off-by: Rony Xavier <[email protected]>

Merge branch 'al/pg_hba_conf' of https://github.com/aaronlippold/inspec…

c098f30

… into al/pg_hba_conf

Merge branch 'al/pg_hba_conf' of https://github.com/aaronlippold/inspec…

7ab6f19

… into al/pg_hba_conf

Merge branch 'al/pg_hba_conf' of https://github.com/aaronlippold/inspec…

3d56503

… into al/pg_hba_conf

Initial commit of pg_hba_conf resource

568237a

Merge branch 'al/pg_hba_conf' of https://github.com/aaronlippold/inspec…

225e3ed

… into al/pg_hba_conf

Corrections

1e6626a

updated to parse auth-options

03a0a8e

Signed-off-by: Aaron Lippold <[email protected]>

updated conf_path instance var to conf_file for consistancy.

e90c560

Signed-off-by: Aaron Lippold <[email protected]>

pg_hba_conf - updated the parse_line method

c9bde40

added test and doc files Signed-off-by: Rony Xavier <[email protected]>

Updated few bugs on pg_hba_conf

ed60ff5

updated test files and docs Signed-off-by: Rony Xavier <[email protected]>

Updated docs

402c247

Signed-off-by: Rony Xavier <[email protected]>

adamhjk added the in progress label Jun 25, 2017

aaronlippold changed the title ~~Al/pg hba conf~~ Postgres pg_hba_conf resource. Jun 25, 2017

aaronlippold force-pushed the al/pg_hba_conf branch 5 times, most recently from 23b4288 to 8179e8c Compare June 25, 2017 20:26

aaronlippold force-pushed the al/pg_hba_conf branch 5 times, most recently from bc6753f to bdbb378 Compare June 26, 2017 03:00

adamleff suggested changes Jun 26, 2017

View reviewed changes

rx294 force-pushed the al/pg_hba_conf branch 2 times, most recently from 8d1d78d to 0089b70 Compare June 26, 2017 21:11

aaronlippold force-pushed the al/pg_hba_conf branch from 0089b70 to bdbb378 Compare June 27, 2017 14:06

adamleff added the Type: New Feature Adds new functionality label Jun 27, 2017

rx294 force-pushed the al/pg_hba_conf branch 3 times, most recently from 656de4d to 8a5f227 Compare June 28, 2017 11:07

Made updates based on the reccomendations

2dff92a

Signed-off-by: Rony Xavier <[email protected]>

rx294 force-pushed the al/pg_hba_conf branch from 8a5f227 to 2dff92a Compare June 28, 2017 11:10

rx294 added 3 commits June 28, 2017 07:11

PR commit

6126121

Signed-off-by: Rony Xavier <[email protected]>

Merge branch 'master' into al/pg_hba_conf

7cb9718

PR Commit

977d471

Signed-off-by: Rony Xavier <[email protected]>

rx294 force-pushed the al/pg_hba_conf branch 2 times, most recently from f9766d0 to 977d471 Compare June 28, 2017 12:05

rx294 added 3 commits June 28, 2017 08:19

Update Gemfile.lock

97d3851

PR Commit

c84fea3

Signed-off-by: Rony Xavier <[email protected]>

Updated doc file for postgres_hba_conf resource to use

3d9a1f8

'cmp' matcher instead of 'eq' Signed-off-by: Rony Xavier <[email protected]>

adamleff changed the title ~~Postgres pg_hba_conf resource.~~ New postgres_hba_conf resource Jun 30, 2017

adamleff approved these changes Jun 30, 2017

View reviewed changes

adamleff requested review from chris-rock and arlimus June 30, 2017 16:29

chris-rock suggested changes Jul 3, 2017

View reviewed changes

Made requested changes, except for SimpleConfig - will address that l…

4aa29ee

…ater. Signed-off-by: Aaron Lippold <[email protected]>

chris-rock approved these changes Jul 3, 2017

View reviewed changes

Merge branch 'master' into al/pg_hba_conf

c1c0325

chris-rock merged commit 224935e into inspec:master Jul 3, 2017

chris-rock removed the in progress label Jul 3, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New postgres_hba_conf resource #1964

New postgres_hba_conf resource #1964

aaronlippold commented Jun 25, 2017

aaronlippold commented Jun 25, 2017 •

edited

Loading

aaronlippold commented Jun 26, 2017

adamleff left a comment

adamleff Jun 26, 2017

rx294 Jun 27, 2017

adamleff Jun 27, 2017

rx294 Jun 28, 2017

adamleff Jun 26, 2017

aaronlippold Jun 26, 2017

adamleff Jun 26, 2017

aaronlippold Jun 26, 2017

adamleff Jun 26, 2017

aaronlippold Jun 26, 2017

chris-rock left a comment

chris-rock Jul 3, 2017

chris-rock Jul 3, 2017

chris-rock Jul 3, 2017

chris-rock Jul 3, 2017

chris-rock left a comment


		An `postgres_hba_conf` InSpec audit resource block declares client authentication data that should be tested:

		describe postgres_hba_conf.where { pg_username == 'filter_value' } do

New postgres_hba_conf resource #1964

New postgres_hba_conf resource #1964

Conversation

aaronlippold commented Jun 25, 2017

aaronlippold commented Jun 25, 2017 • edited Loading

aaronlippold commented Jun 26, 2017

adamleff left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chris-rock left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chris-rock left a comment

Choose a reason for hiding this comment

aaronlippold commented Jun 25, 2017 •

edited

Loading