win_iis_website and win_iis_webbinding - Migrate from community #11

shahargolshani · 2025-01-01T13:07:11Z

SUMMARY

Promote the win_iis_website module from community.windows to microsoft.iis
Promote the win_iis_webbinding module from community.windows to microsoft.iis
Consolidate win_iis_website and win_iis_webbinding modules into microsoft.iis.website

ISSUE TYPE

New Module Pull Request

COMPONENT NAME

plugins/modules/website.ps1
plugins/modules/website.yml
plugins/modules/website_info.ps1
plugins/modules/website_info.yml
tests/integration/targets/website/aliases
tests/integration/targets/website/defaults/main.yml
tests/integration/targets/website/meta/main.yml
tests/integration/targets/website/tasks/bindings.yml
tests/integration/targets/website/tasks/main.yml
tests/integration/targets/website/tasks/tests.yml
tests/integration/targets/website_info/aliases
tests/integration/targets/website_info/defaults/main.yml
tests/integration/targets/website_info/meta/main.yml
tests/integration/targets/website_info/tasks/main.yml
tests/integration/targets/website_info/tasks/tests.yml

ADDITIONAL INFORMATION

Create website (set) module.
Create website_info (get) module.
New integration tests for module microsoft.iis.website
New integration tests for module microsoft.iis.website_info
Updating Documentation files to yml format.

* Fix up docs after migration * Fix up sanity errors

* fix up sanity ignores * Bump ansible-windows dep * Fix bad change for win_region

* Add Extra Information for IIS parameters. hen configuring a website with custom site parameters, for people who are not familiar with PowerShell or IIS, it would be great to add in the documentation some examples to show them how to use these parameters. * Slight tweaks to the docs Co-authored-by: Emir Madrueno Peregrina <[email protected]>

* Rebalance the test targets * Make sure IIS test removes the service so our httptester works

Co-authored-by: Egor Moor <[email protected]>

softwarefactory-project-zuul · 2025-01-01T13:17:44Z

Build succeeded.
https://ansible.softwarefactory-project.io/zuul/buildset/3bd515be0cec482d8587f965e2abc0b9

✔️ ansible-galaxy-importer SUCCESS in 4m 26s
✔️ build-ansible-collection SUCCESS in 5m 15s

softwarefactory-project-zuul · 2025-01-01T13:30:54Z

Build succeeded.
https://ansible.softwarefactory-project.io/zuul/buildset/e94211c4326f4359b7dfa9c0014e5c88

✔️ ansible-galaxy-importer SUCCESS in 4m 30s
✔️ build-ansible-collection SUCCESS in 5m 26s

softwarefactory-project-zuul · 2025-01-01T14:22:16Z

Build succeeded.
https://ansible.softwarefactory-project.io/zuul/buildset/9b4f880994ce411e962304287ca54d1c

✔️ ansible-galaxy-importer SUCCESS in 4m 36s
✔️ build-ansible-collection SUCCESS in 5m 50s

softwarefactory-project-zuul · 2025-01-01T14:45:21Z

Build succeeded.
https://ansible.softwarefactory-project.io/zuul/buildset/37139b9972d641f3b2c131daadd5fbbd

✔️ ansible-galaxy-importer SUCCESS in 10m 28s
✔️ build-ansible-collection SUCCESS in 5m 25s

softwarefactory-project-zuul · 2025-01-01T15:02:23Z

Build succeeded.
https://ansible.softwarefactory-project.io/zuul/buildset/ab2d8cf38dd7459e8a7b28e9e1d7faef

✔️ ansible-galaxy-importer SUCCESS in 4m 57s
✔️ build-ansible-collection SUCCESS in 5m 16s

softwarefactory-project-zuul · 2025-01-01T15:14:42Z

Build succeeded.
https://ansible.softwarefactory-project.io/zuul/buildset/b2c08edb59de4c13b0111f1358b20965

✔️ ansible-galaxy-importer SUCCESS in 3m 22s
✔️ build-ansible-collection SUCCESS in 5m 14s

softwarefactory-project-zuul · 2025-01-01T15:35:49Z

Build succeeded.
https://ansible.softwarefactory-project.io/zuul/buildset/078dd66824d244699797f0b9e0be9316

✔️ ansible-galaxy-importer SUCCESS in 4m 17s
✔️ build-ansible-collection SUCCESS in 5m 19s

softwarefactory-project-zuul · 2025-01-07T15:32:08Z

Build succeeded.
https://ansible.softwarefactory-project.io/zuul/buildset/efc78a39939c4687b1eb04ad14cf951f

✔️ ansible-galaxy-importer SUCCESS in 3m 58s
✔️ build-ansible-collection SUCCESS in 5m 33s

jborean93

This is a pretty large PR. I think we should try and slim it down a bit so we aren't holding up the other PRs.

Let's just focus on getting the bare minimum here of the website and website_info module without the parameters or bindings option implemented. We can focus completely on the simpler options and slowly add back in the other features in a subsequent PR where we can focus more on the UX and what they would look like.

jborean93 · 2025-01-08T03:04:34Z

plugins/modules/website.ps1

+        port = @{ type = 'int' }
+        hostname = @{ type = 'str' }
+        protocol = @{ type = 'str' ; default = 'http' ; choices = @('http', 'https') }
+        ssl_flags = @{ type = 'str' ; default = '0' ; choices = @('0', '1', '2', '3') }


Are these flags representing some more human friendly value. It's hard to tell what each number means without having to look it up.

jborean93 · 2025-01-08T03:08:17Z

plugins/modules/website.ps1

+# Custom site Parameters from string where properties are separated by a pipe and property name/values by colon.
+# Ex. "foo:1|bar:2"
+$parameters = $module.Params.parameters
+if ($null -ne $parameters) {
+    $parameters = @($parameters -split '\|' | ForEach-Object {
+            return , ($_ -split "\:", 2)
+        })
+}


We should remove this format entirely, it was written at a time when modules didn't accept a dictionary. Instead we should do something similar to web_app_pool's attributes option which accepts a dictionary value. We probably need to talk a bit more about trying to align this behavour so the option name stays the same and see if we can try and share the code for getting/setting/comparing the values in a module util.

jborean93 · 2025-01-08T03:08:46Z

plugins/modules/website.ps1

+$module.Result.changed = $false
+
+if ($check_mode) {
+    Write-Output "in check mode"


This will cause warnings and potentially errors as the output will be returned back to ansible which expects just a json string.

jborean93 · 2025-01-08T03:09:58Z

plugins/modules/website.ps1

+    # Add site
+    If (($state -ne 'absent') -and (-not $site)) {
+        If (-not $physical_path) {
+            $module.FailJson("missing required arguments: physical_path $($_.Exception.Message)")


There is no exception here so this can become:

Suggested change

$module.FailJson("missing required arguments: physical_path $($_.Exception.Message)")

$module.FailJson("missing required arguments: physical_path")

jborean93 · 2025-01-08T03:10:17Z

plugins/modules/website.ps1

+            $module.FailJson("missing required arguments: physical_path $($_.Exception.Message)")
+        }
+        ElseIf (-not (Test-Path -LiteralPath $physical_path)) {
+            $module.FailJson("specified folder must already exist: physical_path $($_.Exception.Message)")


Same thing here

Suggested change

$module.FailJson("specified folder must already exist: physical_path $($_.Exception.Message)")

$module.FailJson("specified folder must already exist: physical_path")

jborean93 · 2025-01-08T03:11:29Z

plugins/modules/website.ps1

+        # Change Physical Path if needed
+        if ($physical_path) {
+            If (-not (Test-Path -LiteralPath $physical_path)) {
+                $module.FailJson("specified folder must already exist: physical_path $($_.Exception.Message)")


Suggested change

$module.FailJson("specified folder must already exist: physical_path $($_.Exception.Message)")

$module.FailJson("specified folder must already exist: physical_path")

jborean93 · 2025-01-08T03:14:36Z

plugins/modules/website.ps1

+            $user_bindings = if ($Module.Params.bindings.add) { $Module.Params.bindings.add }
+            elseif ($Module.Params.bindings.remove) { $Module.Params.bindings.remove }
+            elseif ($Module.Params.bindings.set) { $Module.Params.bindings.set }


This logic doesn't sound right to me. It seems like you want to check if set is present and use if, otherwise you need to check both add and remove together to add and remove accordingly.

jborean93 · 2025-01-08T03:14:57Z

plugins/modules/website.ps1

+                    $module.FailJson("SSLFlags can only be set for https protocol")
+                }
+                # Validate certificate details if provided
+                If ($_.certificate_hash -and $_.operation -ne 'remove') {


I don't think $_.operation is defined here.

jborean93 · 2025-01-08T03:15:19Z

plugins/modules/website.ps1

+                # Validate certificate details if provided
+                If ($_.certificate_hash -and $_.operation -ne 'remove') {
+                    If ($_.protocol -ne 'https') {
+                        $module.FailJson("You can  only provide a certificate thumbprint when protocol is set to https")


Suggested change

$module.FailJson("You can only provide a certificate thumbprint when protocol is set to https")

$module.FailJson("You can only provide a certificate thumbprint when protocol is set to https")

jborean93 · 2025-01-08T03:16:40Z

plugins/modules/website.ps1

+    }
+}
+Catch {
+    $module.FailJson("$($module.Result) - $($_.Exception.Message)")


Suggested change

$module.FailJson("$($module.Result) - $($_.Exception.Message)")

$module.FailJson("$($module.Result) - $($_.Exception.Message)", $_)

jborean93 · 2025-01-22T20:29:49Z

Closing as we migrated just the website in #15 and will be doing the binding work in a separate PR.

jborean93 and others added 10 commits March 6, 2020 07:14

Initial commit

a2e938e

Fix up docs after migration (#95)

676de8c

* Fix up docs after migration * Fix up sanity errors

fix up sanity ignores (#97)

f620cce

* fix up sanity ignores * Bump ansible-windows dep * Fix bad change for win_region

Rebalance the test targets (#128)

b1219d5

* Rebalance the test targets * Make sure IIS test removes the service so our httptester works

Remove obsolte ssl parameter (#234)

135cb97

Co-authored-by: Egor Moor <[email protected]>

Fix devel sanity checks - ci_complete (#331)

790753d

Fix new sanity issues (#368)

3d332ad

Bulk semicolon fix for pslint rule (#455)

2717358

Migrate win_iis_website and win_iis_webbinding

1c833e7

shahargolshani force-pushed the feature/migrate-win_iis_website branch from 3e5ab09 to 0a70f48 Compare January 1, 2025 13:20

shahargolshani force-pushed the feature/migrate-win_iis_website branch from 0a70f48 to 7179b56 Compare January 1, 2025 14:10

shahargolshani force-pushed the feature/migrate-win_iis_website branch from 7179b56 to d96bd0e Compare January 1, 2025 14:28

shahargolshani force-pushed the feature/migrate-win_iis_website branch from d96bd0e to 965182d Compare January 1, 2025 14:51

shahargolshani force-pushed the feature/migrate-win_iis_website branch from 965182d to 270beb4 Compare January 1, 2025 15:05

shahargolshani force-pushed the feature/migrate-win_iis_website branch from 270beb4 to ae67968 Compare January 1, 2025 15:25

Migrate win_iis_website and win_iis_webbinding

7a0c42a

shahargolshani force-pushed the feature/migrate-win_iis_website branch from ae67968 to 7a0c42a Compare January 7, 2025 15:21

jborean93 requested changes Jan 8, 2025

View reviewed changes

jborean93 closed this Jan 22, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

win_iis_website and win_iis_webbinding - Migrate from community #11

win_iis_website and win_iis_webbinding - Migrate from community #11

shahargolshani commented Jan 1, 2025

softwarefactory-project-zuul bot commented Jan 1, 2025

softwarefactory-project-zuul bot commented Jan 1, 2025

softwarefactory-project-zuul bot commented Jan 1, 2025

softwarefactory-project-zuul bot commented Jan 1, 2025

softwarefactory-project-zuul bot commented Jan 1, 2025

softwarefactory-project-zuul bot commented Jan 1, 2025

softwarefactory-project-zuul bot commented Jan 1, 2025

softwarefactory-project-zuul bot commented Jan 7, 2025

jborean93 left a comment

jborean93 Jan 8, 2025

jborean93 Jan 8, 2025

jborean93 Jan 8, 2025

jborean93 Jan 8, 2025

jborean93 Jan 8, 2025

jborean93 Jan 8, 2025

jborean93 Jan 8, 2025

jborean93 Jan 8, 2025

jborean93 Jan 8, 2025

jborean93 Jan 8, 2025

jborean93 commented Jan 22, 2025

	$module.FailJson("missing required arguments: physical_path $($_.Exception.Message)")
	$module.FailJson("missing required arguments: physical_path")

	$module.FailJson("specified folder must already exist: physical_path $($_.Exception.Message)")
	$module.FailJson("specified folder must already exist: physical_path")

	$module.FailJson("You can only provide a certificate thumbprint when protocol is set to https")
	$module.FailJson("You can only provide a certificate thumbprint when protocol is set to https")

	$module.FailJson("$($module.Result) - $($_.Exception.Message)")
	$module.FailJson("$($module.Result) - $($_.Exception.Message)", $_)

win_iis_website and win_iis_webbinding - Migrate from community #11

win_iis_website and win_iis_webbinding - Migrate from community #11

Conversation

shahargolshani commented Jan 1, 2025

SUMMARY

ISSUE TYPE

COMPONENT NAME

ADDITIONAL INFORMATION

softwarefactory-project-zuul bot commented Jan 1, 2025

softwarefactory-project-zuul bot commented Jan 1, 2025

softwarefactory-project-zuul bot commented Jan 1, 2025

softwarefactory-project-zuul bot commented Jan 1, 2025

softwarefactory-project-zuul bot commented Jan 1, 2025

softwarefactory-project-zuul bot commented Jan 1, 2025

softwarefactory-project-zuul bot commented Jan 1, 2025

softwarefactory-project-zuul bot commented Jan 7, 2025

jborean93 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

jborean93 commented Jan 22, 2025