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.

Sign up for GitHub

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

azurerm_automation_software_update_configuration - fix windows update classification use a comma separate string #18539

Merged
merged 4 commits into from
Sep 29, 2022
Merged

azurerm_automation_software_update_configuration - fix windows update classification use a comma separate string #18539

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
40e74be
windows update classification use a comma separate string for multipl…
wuxu92 Sep 27, 2022
6d32725
new classifications_included deprecated old one
wuxu92 Sep 29, 2022
8f25ac7
fix classification_included doc
wuxu92 Sep 29, 2022
ce30ade
terrafmt
wuxu92 Sep 29, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 43 additions & 8 deletions internal/services/automation/automation_software_update_configuration.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package automation
import (
"context"
"fmt"
"strings"
"time"

"github.com/Azure/azure-sdk-for-go/services/preview/automation/mgmt/2020-01-13-preview/automation"
Expand Down Expand Up @@ -273,10 +274,13 @@ func targetsFromSDK(prop *automation.TargetProperties) []Target {
}

type Windows struct {
Classification string `tfschema:"classification_included"`
ExcludedKbs []string `tfschema:"excluded_knowledge_base_numbers"`
IncludedKbs []string `tfschema:"included_knowledge_base_numbers"`
RebootSetting string `tfschema:"reboot"`
// Classification Deprecated, use Classifications instead
Classification string `tfschema:"classification_included"`

Classifications []string `tfschema:"classifications_included"`
ExcludedKbs []string `tfschema:"excluded_knowledge_base_numbers"`
IncludedKbs []string `tfschema:"included_knowledge_base_numbers"`
RebootSetting string `tfschema:"reboot"`
}

type SoftwareUpdateConfigurationModel struct {
Expand Down Expand Up @@ -320,8 +324,12 @@ func (s *SoftwareUpdateConfigurationModel) ToSDKModel() automation.SoftwareUpdat

if len(s.Windows) > 0 {
w := s.Windows[0]

upd.Windows = &automation.WindowsProperties{
IncludedUpdateClassifications: automation.WindowsUpdateClasses(w.Classification),
IncludedUpdateClassifications: automation.WindowsUpdateClasses(strings.Join(w.Classifications, ",")),
}
if len(w.Classifications) == 0 && w.Classification != "" {
upd.Windows.IncludedUpdateClassifications = automation.WindowsUpdateClasses(w.Classification)
}

if w.RebootSetting != "" {
Expand Down Expand Up @@ -413,7 +421,12 @@ func (s *SoftwareUpdateConfigurationModel) LoadSDKModel(prop *automation.Softwar
ExcludedKbs: pointer.ToSliceOfStrings(w.ExcludedKbNumbers),
IncludedKbs: pointer.ToSliceOfStrings(w.IncludedKbNumbers),
RebootSetting: utils.NormalizeNilableString(w.RebootSetting),
}}
},
}

for _, v := range strings.Split(string(w.IncludedUpdateClassifications), ",") {
s.Windows[0].Classifications = append(s.Windows[0].Classifications, strings.TrimSpace(v))
}
}

s.Duration = utils.NormalizeNilableString(conf.Duration)
Expand Down Expand Up @@ -506,12 +519,17 @@ func (m SoftwareUpdateConfigurationResource) Arguments() map[string]*pluginsdk.S
"windows": {
Type: pluginsdk.TypeList,
Optional: true,
MaxItems: 1,
Elem: &pluginsdk.Resource{
Schema: map[string]*pluginsdk.Schema{

"classification_included": {
Copy link
Collaborator

Choose a reason for hiding this comment

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

As this is supposed to be a list, and this property is misnamed can we deprecate this and create a new property classifications_included that takes a list and translates it to a CSV list for the api?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have tried with a new property but we have to make it compatible with the deprecated one or there will be diff in the terraform plan. So I have to set both properties as Computed: true to ignore the diff. I don't think it's a so good solution. Is there better practice for this scenario?

Copy link
Collaborator

Choose a reason for hiding this comment

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

yes you will need to set both to computed, always set both, make them conflict, and the use the one that is set. that is the right way to deprecate.

Type: pluginsdk.TypeString,
Optional: true,
Type: pluginsdk.TypeString,
Optional: true,
Computed: true,
ConflictsWith: []string{"windows.0.classifications_included"},
AtLeastOneOf: []string{"windows.0.classification_included", "windows.0.classifications_included"},
Deprecated: "windows classification can be set as a list, use `classifications_included` instead.",
ValidateFunc: validation.StringInSlice(func() (vs []string) {
for _, v := range automation.PossibleWindowsUpdateClassesValues() {
vs = append(vs, string(v))
Expand All @@ -520,6 +538,23 @@ func (m SoftwareUpdateConfigurationResource) Arguments() map[string]*pluginsdk.S
}(), false),
},

"classifications_included": {
Type: pluginsdk.TypeList,
Optional: true,
Computed: true,
ConflictsWith: []string{"windows.0.classification_included"},
AtLeastOneOf: []string{"windows.0.classification_included", "windows.0.classifications_included"},
Elem: &pluginsdk.Schema{
Type: pluginsdk.TypeString,
ValidateFunc: validation.StringInSlice(func() (res []string) {
for _, v := range automation.PossibleWindowsUpdateClassesValues() {
res = append(res, string(v))
}
return
}(), false),
},
},

"excluded_knowledge_base_numbers": {
Type: pluginsdk.TypeList,
Optional: true,
Expand Down
72 changes: 72 additions & 0 deletions internal/services/automation/automation_software_update_configuration_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,21 @@ func TestAccSoftwareUpdateConfiguration_update(t *testing.T) {
})
}

func TestAccSoftwareUpdateConfiguration_windows(t *testing.T) {
data := acceptance.BuildTestData(t, automation.SoftwareUpdateConfigurationResource{}.ResourceType(), "test")
r := newSoftwareUpdateConfigurationResource()
data.ResourceTest(t, r, []acceptance.TestStep{
{
Config: r.windows(data),
Check: acceptance.ComposeTestCheckFunc(
check.That(data.ResourceName).ExistsInAzure(r),
),
},
// scheduleInfo.advancedSchedule always return null
data.ImportStep("schedule.0.advanced", "schedule.0.monthly_occurrence"),
})
}

func (a SoftwareUpdateConfigurationResource) basic(data acceptance.TestData) string {
return fmt.Sprintf(`

Expand Down Expand Up @@ -194,6 +209,63 @@ resource "azurerm_automation_software_update_configuration" "test" {
`, a.template(data), data.RandomInteger, a.startTime, a.expireTime)
}

func (a SoftwareUpdateConfigurationResource) windows(data acceptance.TestData) string {
return fmt.Sprintf(`


%s

resource "azurerm_automation_software_update_configuration" "test" {
automation_account_id = azurerm_automation_account.test.id
name = "acctest-suc-%[2]d"
operating_system = "Windows"

windows {
classifications_included = ["Critical", "Security"]
reboot = "IfRequired"
}

duration = "PT1H1M1S"
virtual_machine_ids = []

target {
azure_query {
scope = [azurerm_resource_group.test.id]
locations = [azurerm_resource_group.test.location]
tags {
tag = "foo"
values = ["barbar2"]
}
tag_filter = "Any"
}

non_azure_query {
function_alias = "savedSearch1"
workspace_id = azurerm_log_analytics_workspace.test.id
}
}

schedule {
description = "foo-schedule"
start_time = "%[3]s"
expiry_time = "%[4]s"
is_enabled = true
interval = 1
frequency = "Hour"
time_zone = "Etc/UTC"
advanced_week_days = ["Monday", "Tuesday"]
advanced_month_days = [1, 10, 15]
monthly_occurrence {
occurrence = 1
day = "Tuesday"
}
}

depends_on = [azurerm_log_analytics_linked_service.test]
}
`, a.template(data), data.RandomInteger, a.startTime, a.expireTime)
}

// software update need log analytic location map correct, if use a random location like `East US` will cause
// error like `chosen Azure Automation does not have a Log Analytics workspace linked for operation to succeed`.
// so location hardcode as `West US`
Expand Down
4 changes: 3 additions & 1 deletion website/docs/r/automation_software_update_configuration.html.markdown
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,9 @@ A `linux` block supports the following:

A `windows` block supports the following:

* `classification_included` - (Optional) Specifies the update classification. Possible values are `Unclassified`, `Critical`, `Security`, `UpdateRollup`, `FeaturePack`, `ServicePack`, `Definition`, `Tools` and `Updates`.
* `classification_included` - (Deprecated) Specifies the update classification. Possible values are `Unclassified`, `Critical`, `Security`, `UpdateRollup`, `FeaturePack`, `ServicePack`, `Definition`, `Tools` and `Updates`.

* `classifications_included` - (Optional) Specifies the list of update classification. Possible values are `Unclassified`, `Critical`, `Security`, `UpdateRollup`, `FeaturePack`, `ServicePack`, `Definition`, `Tools` and `Updates`.

* `excluded_knowledge_base_numbers` - (Optional) Specifies a list of knowledge base numbers excluded.

Expand Down