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

Update iis_site bindingInformation construction and add tests #3492

Merged
merged 1 commit into from
Nov 8, 2018

Conversation

mrshanahan
Copy link
Contributor

@mrshanahan mrshanahan commented Oct 11, 2018

Signed-off-by: Matt Shanahan mrshanahan11235@gmail.com

I have not made any changes to the docs, since this brings the code in line with the documentation. Also added some unit tests to the iis_site resource.

Relevant resource docs: https://github.com/inspec/inspec/blob/master/docs/resources/iis_site.md.erb#L109-L139

Fixes #3490
EDIT (@clintoncwolfe): Move reference to issue from PR title to description to trigger close-on-merge.

…c#3490)

Signed-off-by: Matt Shanahan <mrshanahan11235@gmail.com>
@aaronlippold
Copy link
Collaborator

hi, given that you document at least twice that you only support 2012+, would it be a good idea to add that as part of the platforms lotic in the init method of the resource?

@mrshanahan
Copy link
Contributor Author

@aaronlippold That makes sense! Let me take a look.

@miah
Copy link
Contributor

miah commented Oct 17, 2018

@mrshanahan, @aaronlippold

We're moving away from doing platform support restrictions in initialize. This should all be handled by the supports dsl now.

We could do something like:

supports platform: 'windows', release: '6.2.*'

Where release is the right version number =) Note that you can add multiple supports, so you should be able to cover all the platforms.

@aaronlippold
Copy link
Collaborator

I agree for the profile for iis. But for the initialization of the iis resource itself shouldn't we still have that fail fast?

@mrshanahan
Copy link
Contributor Author

mrshanahan commented Oct 29, 2018

I'm happy to add the appropriate supports statements to narrow down the acceptable list of platforms, but I'm not sure how to find what the available platform names are. Can I select e.g. windows_server instead of windows? I had some trouble finding the relevant code.

@jquick
Copy link
Contributor

jquick commented Oct 29, 2018

@mrshanahan , The windows specific names ones are a bit dynamic and end up being something like windows_server_2012_r2_standard_evaluation. I would stick with something like what @miah suggested. The release portion should contain it only to the ones we want.

We use this command to find the windows release https://github.com/inspec/train/blob/master/lib/train/platforms/detect/helpers/os_windows.rb#L6

Copy link
Contributor

@clintoncwolfe clintoncwolfe left a comment

Choose a reason for hiding this comment

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

Thanks so much! This is a great first contribution, and we really appreciate it. There is some concern about restricting the releases on which the resource will run, but your PR is independent of that concern; I've opened #3591 to track needing to tighten the platform support, and that clears the way to get this merged in.

Thanks!

@clintoncwolfe clintoncwolfe changed the title Update iis_site bindingInformation construction and add tests. (#3490) Update iis_site bindingInformation construction and add tests Nov 8, 2018
@clintoncwolfe clintoncwolfe added Type: Bug Feature not working as expected Component: Core Resources Resources shipped with InSpec. labels Nov 8, 2018
Copy link
Contributor

@jquick jquick left a comment

Choose a reason for hiding this comment

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

Thanks @mrshanahan

@jquick jquick merged commit cebe044 into inspec:master Nov 8, 2018
@mrshanahan
Copy link
Contributor Author

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Core Resources Resources shipped with InSpec. Platform: Windows Windows-specific issues Type: Bug Feature not working as expected
Projects
None yet
Development

Successfully merging this pull request may close these issues.

iis_site: bindings property not populated correctly
5 participants