(GH-8)(GH-9)(GH10) added additional parameters, update docs #6

ripclawffb · 2016-09-27T00:47:20Z

include support for previous module implementations
added ability to not disable the default website
added ability to change packages folder

Closes #8, #9, #10

ferventcoder

Thanks for the contribution! However I have some questions regarding the switch to DSC - which is typically done when there is a feature necessary that you don't find in the native Puppet resource option. I'm not seeing it here so please elaborate more on why the switch.

ferventcoder · 2016-10-02T15:08:22Z

README.md

+website. Defaults to 'C:\inetpub\wwwroot'.
+
+##### `packages_folder`
+An alternate folder can be defined for the .nupkg files.


This should already be possible - see server_package_source (that's what it is for, you can set it to c:\packages).

I didn't see my previous comment for some reason. From my understanding, server_package_source only specifies where the chocolatey.server nuget package can be found. This setting allows you to change the path of where the nuget packages are served from instead of the default C:\tools\chocolatey.server\App_Data\Packages.

Fair statement! I didn't realize that this is what set this.

ferventcoder · 2016-10-02T15:09:00Z

README.md

+##### `packages_folder_permissions`
+Set permissions on packages folder. Defaults to:
+  * IIS APPOOL\${chocolatey_server_app_pool_name} - modify
+  * IIS_IUSRS - modify


Interesting, how is this actually passed? Would prefer an example of this one in the readme.

Sure, I can update the readme with examples to make it more clear.. This is passed as an array of permissions as a parameter, similar to how you set the permissions for the chocolatey.server folder.

I have added an example that shows how to pass an array of permissions to the readme. Let me know if more clarity is needed.

ferventcoder · 2016-10-02T15:09:13Z

README.md

+The default website is stopped to prevent conflicts with the
+chocolatey server website. An alternate port can be defined and
+this option can be set to disabled so it doesn't modify the default
+website. Defaults to 'true'.


I like this.

ferventcoder · 2016-10-02T15:11:01Z

README.md

+
+##### `default_website_path`
+The default website path is needed in order to modify the default
+website. Defaults to 'C:\inetpub\wwwroot'.


I'm not sure this is necessary with WindowsFeatures module - I'm hesitant to convert to Puppet+DSC as the native option is there. I will explain why native option is better in my review comments at the end.

I removed the dsc dependency as requested.

ferventcoder · 2016-10-02T15:22:21Z

README.md

@@ -104,6 +114,32 @@ Host your own Chocolatey package repository

 #### Parameters

+##### `chocolatey_server_app_pool_name`
+Set apppool name used by the chocolatey.server website. Defaults to
+'chocolatey.server'.


ferventcoder · 2016-10-02T15:25:10Z

metadata.json

+      "name": "puppetlabs/dsc",
+      "version_requirement": ">= 0.8.0 < 2.0.0"
+    },
+    {


Not a fan of adding a dependency unless necessary.

Removed as requested. :)

ferventcoder · 2016-10-02T15:27:24Z

manifests/init.pp

+    dsc_xwebapppool { "${_chocolatey_server_app_pool_name}":
+      dsc_ensure                => present,
+      dsc_name                  => "${_chocolatey_server_app_pool_name}",
+      dsc_enable32bitapponwin64 => false,


This was set to true previously, why the switch?

I tested this again, and it looks like it does work as-is, so I removed the change for disabling the 32-bit app_pool mode.

ferventcoder · 2016-10-02T15:28:54Z

Whenever there is a native Puppet resource and a Puppet+DSC resource option, it is typically better to prefer the native resource as it handles "in sync" better and is closer to the metal (not a lot of abstractions to go through means less places to look when things are not working).

The "in sync" issue is typically due to many DSC resources ignoring one of the three options (Get/Set/Test, I can not remember offhand which it is currently, it's either Get or Test, I think it is Get) and that causes conflicts when determining if a resource is already set appropriately. We worked hard to work around this limitation with puppetlabs-dsc, but it sometimes requires more time to make that determination.

Another reason to prefer the native resource is typically the Puppet resource can be ensurable, which because of the "in sync" issue is not something that can be done with Puppet+DSC.

Another good reason to prefer the native resource option is better output in what is changing (the diffs you see when Puppet corrects), again due to the same issue of "in sync".

So unless there is an option available in the DSC resource that is not available in the Puppet native option, it is best to prefer the Puppet resource.

Perhaps you can help me understand why your preference is for the DSC resources here?

ripclawffb

Internally, we're starting to use DSC more since over time, more items are available to be managed. I figured this would lead to less dependencies in the long run. However, if you're still inclined to use native types, I can modify my changes and remove the dsc types. Thanks for the feedback.

ferventcoder · 2016-10-02T16:52:51Z

Hopefully over time those DSC resources will have better implemented the ability to check the values they have to meet the insync and ensurable requirements that Puppet needs. Unfortunately it was not a focus for folks and guidance from Microsoft was not to worry about it when building DSC resources. I think they've started to change that stance more now though and that is a good thing for everyone 👍

For now though, it would probably be best to stick with the native resources, since they are approved modules - this module is neither approved nor supported, which means you are likely open to using approved modules (or at least this one! Ha :D ).

ripclawffb · 2016-10-02T19:00:01Z

README.md

+##### `chocolatey_server_app_pool_name`
+Set apppool name used by the chocolatey.server website. Defaults to
+'chocolatey.server'.
+


I don't think it is, as someone else submitted a merge request previously that used the windows feature module and it didn't look like they needed to supply the default website path.

ripclawffb · 2016-10-02T19:02:53Z

README.md

+##### `packages_folder_permissions`
+Set permissions on packages folder. Defaults to:
+  * IIS APPOOL\${chocolatey_server_app_pool_name} - modify
+  * IIS_IUSRS - modify


Sure, I can update the readme with examples to make it more clear.. This is passed as an array of permissions as a parameter, similar to how you set the permissions for the chocolatey.server folder.

ripclawffb · 2016-10-02T19:08:34Z

README.md

+this option can be set to disabled so it doesn't modify the default
+website. Defaults to 'true'.
+
+##### `packages_folder`


If I read the code correctly, the server_package_source is the location where only the chocolatey.server nuget package can be found. This setting allows you to remap the c:\tools\chocolatey.server\app_data\packages folder to another location where the actual nuget packages being served are stored. For instance, we store all of our nuget packages in c:\packages. Hope that makes sense.

That makes more sense.

ripclawffb

Made the requested changes, updated dependencies and docs.

ripclawffb · 2016-10-02T21:51:34Z

README.md

+
+##### `default_website_path`
+The default website path is needed in order to modify the default
+website. Defaults to 'C:\inetpub\wwwroot'.


I removed the dsc dependency as requested.

ripclawffb · 2016-10-02T21:52:58Z

README.md

+##### `packages_folder_permissions`
+Set permissions on packages folder. Defaults to:
+  * IIS APPOOL\${chocolatey_server_app_pool_name} - modify
+  * IIS_IUSRS - modify


I have added an example that shows how to pass an array of permissions to the readme. Let me know if more clarity is needed.

ripclawffb · 2016-10-02T21:54:33Z

README.md

+website. Defaults to 'C:\inetpub\wwwroot'.
+
+##### `packages_folder`
+An alternate folder can be defined for the .nupkg files.


I didn't see my previous comment for some reason. From my understanding, server_package_source only specifies where the chocolatey.server nuget package can be found. This setting allows you to change the path of where the nuget packages are served from instead of the default C:\tools\chocolatey.server\App_Data\Packages.

ripclawffb · 2016-10-02T21:55:33Z

manifests/init.pp

+    dsc_xwebapppool { "${_chocolatey_server_app_pool_name}":
+      dsc_ensure                => present,
+      dsc_name                  => "${_chocolatey_server_app_pool_name}",
+      dsc_enable32bitapponwin64 => false,


I tested this again, and it looks like it does work as-is, so I removed the change for disabling the 32-bit app_pool mode.

ripclawffb · 2016-10-02T21:55:58Z

metadata.json

+      "name": "puppetlabs/dsc",
+      "version_requirement": ">= 0.8.0 < 2.0.0"
+    },
+    {


Removed as requested. :)

ferventcoder · 2016-10-05T22:20:27Z

README.md

+nuget packages are actually stored that will be served by the chocolatey server.
+
+Note: This is different from 'server_package_source' as that defines where
+the chocolatey.server package is located.


ripclawffb · 2016-10-05T23:07:18Z

Requested changes completed.

ferventcoder · 2016-10-12T15:56:47Z

Please do not use merge. Always rebase. Also check your username/email for commits. Looks like you have possibly flipped to a different user somewhere.

ferventcoder · 2016-10-12T19:43:37Z

That definitely looks better. 👍

ferventcoder · 2016-10-12T19:46:08Z

manifests/init.pp

+      { identity => 'IUSR',
+      rights     => ['read'] },
+      { identity => "IIS APPPOOL\\${_chocolatey_server_app_pool_name}",
+      rights     => ['read'] }


Lint hates the way I had it, so thanks for that. I'm much more a fan of it getting it so that lint ignores these lines (not a fan of dropping lines here as it makes it harder to read each permission). No worries though.

ferventcoder · 2016-10-14T18:12:53Z

manifests/init.pp

-    ensure    => absent,
-    site_path => 'any',
-    app_pool  => 'DefaultAppPool',
-    require   => Windowsfeature['Web-WebServer'],


For me during final pull in - looks like this line was lost.

Also note that if you just include chocolatey_server it will disable instead of delete the default website. 👍

That's on purpose. Is that ok? I think it's better to disable the website than to delete it.

ferventcoder · 2016-10-14T18:27:04Z

manifests/init.pp

+      'allowOverrideExistingPackageOnPush' => $allow_package_override,
+      'apiKey'                             => $apikey,
+      'requireApiKey'                      => $require_apikey,
+    }),


Just realized this was EPP and not ERB. It requires future parser in 3.5+ or Puppet v4+. I'm not against using EPP, but hoping to allow this to use erb instead for a bit further compatibility. Is there any disadvantage to using ERB?

Sure, I can try to rework it into an ERB template. I'll let you know when complete.

I have converted the EPP template to an ERB template as requested.

Add additional parameters to modify the chocolatey repository package folder and set custom permissions. Update readme.

Change default behavior to stop default website instead of deleting it. Update readme.

Add parameters to allow modification of the default chocolatey server config such as the API key and the ability to overwrite existing packages. Update readme

Update dependencies for resources. Upgrade windowsfeature module version.

ripclawffb · 2018-04-23T04:38:33Z

@ferventcoder I went ahead and rebased against latest master and tested the changes. Everything seems to be working as expected. Can you please review and merge?

ripclawffb changed the title ~~added additional parameters, update docs~~ added additional parameters, use dsc for website and features, update docs Sep 27, 2016

ferventcoder requested changes Oct 2, 2016

View reviewed changes

ripclawffb commented Oct 2, 2016

View reviewed changes

ripclawffb force-pushed the default_website_option branch from 40f330c to 68351ba Compare October 2, 2016 21:44

ripclawffb commented Oct 2, 2016

View reviewed changes

ripclawffb force-pushed the default_website_option branch from d4401d4 to d37290f Compare October 4, 2016 03:59

ferventcoder reviewed Oct 5, 2016

View reviewed changes

ripclawffb changed the title ~~added additional parameters, use dsc for website and features, update docs~~ added additional parameters, update docs Oct 5, 2016

ripclawffb force-pushed the default_website_option branch 4 times, most recently from f039073 to 7b68d10 Compare October 11, 2016 23:04

ripclawffb force-pushed the default_website_option branch 2 times, most recently from e5cb3a9 to b08bcd1 Compare October 12, 2016 19:40

ferventcoder reviewed Oct 12, 2016

View reviewed changes

ferventcoder changed the title ~~added additional parameters, update docs~~ (GH-8)(GH-9)(GH10) added additional parameters, update docs Oct 14, 2016

ferventcoder reviewed Oct 14, 2016

View reviewed changes

ripclawffb force-pushed the default_website_option branch from ddf67d3 to 9603714 Compare October 15, 2016 21:22

ferventcoder approved these changes Feb 22, 2017

View reviewed changes

ripclawffb and others added 4 commits April 21, 2018 23:22

(chocolatey-communityGH-10) Add support for custom package folder

b2c17ad

Add additional parameters to modify the chocolatey repository package folder and set custom permissions. Update readme.

(chocolatey-communityGH-8) Add support for default website

5914994

Change default behavior to stop default website instead of deleting it. Update readme.

(chocolatey-communityGH-9) Add support to customize chocolatey server

19f2b70

Add parameters to allow modification of the default chocolatey server config such as the API key and the ability to overwrite existing packages. Update readme

(maint) Update dependencies and windowsfeature dependency

64947c8

Update dependencies for resources. Upgrade windowsfeature module version.

ripclawffb force-pushed the default_website_option branch 3 times, most recently from c7e103a to 27fda2b Compare April 23, 2018 03:21

(maint) Change template to erb and fix duplicate declarations

e87ad97

ripclawffb force-pushed the default_website_option branch from 27fda2b to e87ad97 Compare April 23, 2018 04:24

ripclawffb closed this Nov 3, 2020

(GH-8)(GH-9)(GH10) added additional parameters, update docs #6

(GH-8)(GH-9)(GH10) added additional parameters, update docs #6

Conversation

ripclawffb commented Sep 27, 2016 • edited by ferventcoder Loading

ferventcoder 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

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

ferventcoder commented Oct 2, 2016

ripclawffb left a comment

Choose a reason for hiding this comment

ferventcoder commented Oct 2, 2016

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

ripclawffb 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

ripclawffb commented Oct 5, 2016

ferventcoder commented Oct 12, 2016 • edited Loading

ferventcoder commented Oct 12, 2016

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

ripclawffb commented Apr 23, 2018

ripclawffb commented Sep 27, 2016 •

edited by ferventcoder

Loading

ferventcoder commented Oct 12, 2016 •

edited

Loading