Refactor vdir name property to site_name #359

fdzedzy · 2017-05-15T18:14:48Z

Description

Refactoring the name property on the vdir resource to be site_name. This mimics the site_name property on the app resource. This will allow multiple vdirs to be created on a single site without creating a resource cloning error in Chef 13. Using a property called name will cause a property name collision.

This would create two virtual directories on the same site without throwing any Chef 13 warnings:

iis_vdir 'vdir1' do
  site_name 'Default Web Site/'
  path '/vdir_test1'
  physical_path "#{node['iis']['docroot']}\\vdir_test1"
  action [:add, :config]
end

iis_vdir 'vdir2' do
  site_name 'Default Web Site/'
  path '/vdir_test2'
  physical_path "#{node['iis']['docroot']}\\vdir_test2"
  action [:add, :config]
end

This change also creates consistency between the app and vdir resources.
Tests have been updated to match the app resource.

Check List

All tests pass. See https://github.com/chef-cookbooks/community_cookbook_documentation/blob/master/TESTING.MD
New functionality includes testing.
New functionality has been documented in the README if applicable
All commits have been signed for the Developer Certificate of Origin. See https://github.com/chef-cookbooks/community_cookbook_documentation/blob/master/CONTRIBUTING.MD

Signed-off-by: fdzedzy <fdzedzy@frontlinetechnologies.com>

Signed-off-by: Justin Schuhmann <jmschu02@gmail.com>

Fix tests for iis_pool

Signed-off-by: fdzedzy <fdzedzy@frontlinetechnologies.com>

EasyAsABC123 · 2017-05-15T19:34:46Z

this was changed from application_name to name because this is actually the

This is the name of the website or site + application you are adding it to.

Which isn't just the site name or the application name but a possible combination of both, technically this is referred to as the location or path. I went with name just to stop people from being confused

EasyAsABC123 · 2017-05-15T19:38:33Z

I agree this should change since there is a bug in chef 13 causing the before mentioned error, but i'd say we go with application_name since that's what it is called in appcmd

appcmd add vdir /app.name:contoso/marketing /path:/photos /physicalPath:c:\images

EasyAsABC123

mainly let's go with application_name since it matches the old name for it and the appcmd name for it.

EasyAsABC123 · 2017-05-15T19:39:37Z

README.md

@@ -397,7 +397,7 @@ Allows easy management of IIS virtual directories (i.e. vdirs).

 #### Attribute Parameters

- `name`: name attribute. Specifies the value of the name attribute. This is the name of the website or site + application you are adding it to.
+- `site_name`: name attribute. This is the name of the website or site + application you are adding it to.


let's use application_name this will add backwards compatibility back and not make this more confusing since app.name is the property used in appcmd

EasyAsABC123 · 2017-05-15T19:41:03Z

test/integration/vdir/libraries/iis_vdir.rb

@@ -29,9 +29,9 @@ class IisVdir < Inspec.resource(1)
 end
 "

- def initialize(name, path)
- @name = name
+ def initialize(path, site_name)


no reason to invert these, just leave them as is...also change to application name if desired although no required

I inverted them to be consistent with the app resource.

fdzedzy · 2017-05-15T21:06:15Z

The problem I ran into with name is that it is also a ruby method. I picked site_name to match the attribute also used in the app resource. Virtual applications and virtual directories are both created under a site so they both seem similar in that regards.

EasyAsABC123 · 2017-05-15T21:20:21Z

Virtual directories go onto site and applications, that's the issue. I'm down with this change but it should be application_name since that is the same as appcmd

fdzedzy · 2017-05-15T21:25:37Z

Ah, I see what you mean. That makes sense. I'll update that so it is more clear.

EasyAsABC123 · 2017-05-15T21:26:54Z

Awesome thanks! And good work catching this. Was this something that you could make fail in a test before your changes and work after?

Signed-off-by: fdzedzy <fdzedzy@frontlinetechnologies.com>

fdzedzy · 2017-05-16T12:51:24Z

I've got a cookbook that sets up a bunch of apps and virtual directories under a single site with a lot of unique configuration so there were several changes needed to use the new resources. In testing for Chef 13 compatibility I was resolving the duplicate resource issue and my cookbook wouldn't converge when I tried to set the name property because it said it was a Ruby method.

EasyAsABC123 · 2017-05-16T20:49:51Z

@fdzedzy that's weird...hurm, well renaming it should solve that. With the scripts you have can you add the failing items to the unit tests that already exists? basically add it to:
https://github.com/chef-cookbooks/iis/blob/master/test/integration/vdir/controls/vdir_spec.rb
https://github.com/chef-cookbooks/iis/blob/master/test/cookbooks/test/recipes/vdir.rb

fdzedzy · 2017-05-17T14:57:57Z

@EasyAsABC123 Just to follow up, the test you added to cover the trailing slash PR should cover the scenario in this PR. I was thinking of adding another test to add a virtual directory to an application under a default site since this scenario wasn't covered. I can submit a new PR with that test. I'm wondering if that change to not require the / broke the scenario of adding a vdir to an app. Not sure if that is just my cookbook or a real issue yet.

Most of the merge work is based on the fact that pool provider has been replaced by a custom resource. From all the non-merged criteo patches: 51e609a Fix decoding of entities in XML 5034448 Clarify execute tasks 43186f1 Activate use_inline_resources 99c4187 Remove unnecessary Chef logs 280fba8 Fix regression introduced with whyrun support e85ca2f Fix the site_identifier privacy so that the execute context can use it 12ac82f Post review fixes d534851 Removed new_resource.updated_by_last_action occurences f9b773d Make the pool provider whyrun able all are either unnecessary or correctly applied * 'master' of https://github.com/chef-cookbooks/iis: (145 commits) Adds better documentation for the options parameter (sous-chefs#383) release 6.7.2 (sous-chefs#382) Fix FTP site binding error (sous-chefs#380) Release v6.7.1 (sous-chefs#379) Fix issue with guard clause missing on check (sous-chefs#378) Release v6.7.0 (sous-chefs#376) fix multiple bugs with iis_vdir iis_root iis_app for idempotency (sous-chefs#375) Release v6.6.0 (sous-chefs#373) Update appveyor.yml Enhancement module custom resource (sous-chefs#372) Fix README.md (sous-chefs#367) Release v6.5.3 (sous-chefs#371) Resolves a bug in iis_vdir also adds more liberty in config (sous-chefs#370) Refactor vdir name property to site_name (sous-chefs#359) Release v6.5.2 (sous-chefs#364) Update iis_vdir name to not require a trailing / (sous-chefs#363) Fix iis_pool identity_type issue (sous-chefs#362) Release v6.5.1 (sous-chefs#358) Add quotes to pool name (sous-chefs#357) Release v6.5.0 (sous-chefs#352) ... Change-Id: I951fb2d1788d877c98b24bb22cb5d035162e146d

fdzedzy and others added 11 commits May 12, 2017 14:11

Fixing pool name spaces bug

08974a4

Signed-off-by: fdzedzy <fdzedzy@frontlinetechnologies.com>

setting pool test to have space in name

b7b4975

Signed-off-by: fdzedzy <fdzedzy@frontlinetechnologies.com>

merging in master

8b57872

Signed-off-by: fdzedzy <fdzedzy@frontlinetechnologies.com>

Fix tests for iis_pool

91b2f81

Signed-off-by: Justin Schuhmann <jmschu02@gmail.com>

Merge pull request #1 from EasyAsABC123/master

96e7824

Fix tests for iis_pool

Refactor vdir name to site_name

4e90162

Signed-off-by: fdzedzy <fdzedzy@frontlinetechnologies.com>

Merge branch 'master' of https://github.com/fdzedzy/iis

4402205

Update to tests

08164b4

Signed-off-by: fdzedzy <fdzedzy@frontlinetechnologies.com>

Merge branch 'master' of https://github.com/chef-cookbooks/iis

64d54cf

Cleaning up testing code

f22d8d4

Signed-off-by: fdzedzy <fdzedzy@frontlinetechnologies.com>

Fixing unit tests

86252b3

Signed-off-by: fdzedzy <fdzedzy@frontlinetechnologies.com>

EasyAsABC123 self-requested a review May 15, 2017 19:34

EasyAsABC123 added the Status: Maintainer Review Needed label May 15, 2017

EasyAsABC123 suggested changes May 15, 2017

View reviewed changes

fdzedzy added 4 commits May 16, 2017 07:52

changing site_name to application_name

5dea201

Signed-off-by: fdzedzy <fdzedzy@frontlinetechnologies.com>

Resolving merge conflicts

0a3e724

Signed-off-by: fdzedzy <fdzedzy@frontlinetechnologies.com>

Fixing unit test

c019cc5

Signed-off-by: fdzedzy <fdzedzy@frontlinetechnologies.com>

Switching order of properties in spec to pass unit tests

70674d8

Signed-off-by: fdzedzy <fdzedzy@frontlinetechnologies.com>

EasyAsABC123 approved these changes May 17, 2017

View reviewed changes

EasyAsABC123 merged commit 75afae9 into sous-chefs:master May 17, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor vdir name property to site_name #359

Refactor vdir name property to site_name #359

fdzedzy commented May 15, 2017

EasyAsABC123 commented May 15, 2017 •

edited

Loading

EasyAsABC123 commented May 15, 2017

EasyAsABC123 left a comment

EasyAsABC123 May 15, 2017

EasyAsABC123 May 15, 2017

fdzedzy May 16, 2017

fdzedzy commented May 15, 2017

EasyAsABC123 commented May 15, 2017

fdzedzy commented May 15, 2017

EasyAsABC123 commented May 15, 2017

fdzedzy commented May 16, 2017

EasyAsABC123 commented May 16, 2017

fdzedzy commented May 17, 2017

Refactor vdir name property to site_name #359

Refactor vdir name property to site_name #359

Conversation

fdzedzy commented May 15, 2017

Description

Check List

EasyAsABC123 commented May 15, 2017 • edited Loading

EasyAsABC123 commented May 15, 2017

EasyAsABC123 left a comment

Choose a reason for hiding this comment

EasyAsABC123 May 15, 2017

Choose a reason for hiding this comment

EasyAsABC123 May 15, 2017

Choose a reason for hiding this comment

fdzedzy May 16, 2017

Choose a reason for hiding this comment

fdzedzy commented May 15, 2017

EasyAsABC123 commented May 15, 2017

fdzedzy commented May 15, 2017

EasyAsABC123 commented May 15, 2017

fdzedzy commented May 16, 2017

EasyAsABC123 commented May 16, 2017

fdzedzy commented May 17, 2017

EasyAsABC123 commented May 15, 2017 •

edited

Loading