Skip to content

monitoring: add write-only support for notification_channel sensitive_labels #15524

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

Closed
GraceAtwood wants to merge 2 commits into from
Closed

monitoring: add write-only support for notification_channel sensitive_labels #15524

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c1ed183
Add write-only support for monitoring_notification_channel sensitive_…
GraceAtwood Oct 28, 2025
38b62b0
Add tests and fix write-only field attributes for notification_channel
GraceAtwood Nov 18, 2025
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
57 changes: 57 additions & 0 deletions mmv1/products/monitoring/NotificationChannel.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,28 +108,85 @@ properties:
description: |
An authorization token for a notification channel. Channel types that support this field include: slack
sensitive: true
conflicts:
- 'sensitive_labels.0.auth_token_wo'
exactly_one_of:
- 'sensitive_labels.0.auth_token'
- 'sensitive_labels.0.password'
- 'sensitive_labels.0.service_key'
- name: 'authTokenWo'
Copy link
Collaborator

@BBBmau BBBmau Nov 11, 2025

Choose a reason for hiding this comment

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

explcitily adding the write-only isn't needed. You can add write-only support by adding write_only: true on the desired field in this case: authToken

Copy link
Contributor

@ramonvermeulen ramonvermeulen Nov 12, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

@BBBmau @melinath see #15538 (comment), unfortunately this isn't fully supported yet for fields with constraint groups. The PR I linked solves the constraint group issues, but while writing tests I encountered some other issues, because it is a nested field and url_param_only is set to true for this specific sensetive_labels implementation. I will see what I can do, because the goal is to make write_only: true generation work for any type of write-only use-case.

Copy link
Author

@GraceAtwood GraceAtwood Nov 18, 2025

Choose a reason for hiding this comment

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

Am I following correctly then that there's no change I need to make here?

Copy link
Member

@melinath melinath Nov 18, 2025

Choose a reason for hiding this comment

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

We'll only want to move forward with one of the PRs, and we'll definitely want to use the new write_only rather than write_only_legacy.

I'd ideally like to separate the general "handling of nested write_only fields" changes from "handling of notification_channel sensitive labels" to make things easier to review / less risky.

Given these are both community contributions (thanks both of you!) we could go either way. Currently I'd be inclined to stick with @ramonvermeulen's PR since it's pretty far along, but I could be convinced otherwise.

Copy link
Member

@melinath melinath Nov 18, 2025

Choose a reason for hiding this comment

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

@c2thorn @BBBmau what do you think?

Copy link
Member

@c2thorn c2thorn Nov 18, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Spoke offline with @melinath and we prefer to move forward with #15538 as it spans the changes made here but in the preferred way. We'll move that PR higher in prioritization. There are still things to work out, but there is a release freeze for the next two weeks anyway.

Copy link
Member

@c2thorn c2thorn Nov 18, 2025

Choose a reason for hiding this comment

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

@GraceAtwood appreciate your patience and efforts on this PR. At this point this can be closed in favor of #15538.

Copy link
Author

@GraceAtwood GraceAtwood Nov 20, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Sounds good thanks! closed this one

type: String
description: |
An authorization token for a notification channel. Channel types that support this field include: slack. For more info see [updating write-only attributes](/docs/providers/google/guides/using_write_only_attributes.html#updating-write-only-attributes)
write_only_legacy: true
required_with:
- 'authTokenWoVersion'
conflicts:
- 'sensitive_labels.0.auth_token'
- name: 'authTokenWoVersion'
type: Integer
immutable: true
ignore_read: true
description: |
Triggers a new auth_token_wo to be written. Increment this to update auth_token_wo. For more info see [updating write-only attributes](/docs/providers/google/guides/using_write_only_attributes.html#updating-write-only-attributes)
required_with:
- 'sensitive_labels.0.auth_token_wo'
- name: 'password'
type: String
description: |
An password for a notification channel. Channel types that support this field include: webhook_basicauth
sensitive: true
conflicts:
- 'sensitive_labels.0.password_wo'
exactly_one_of:
- 'sensitive_labels.0.auth_token'
- 'sensitive_labels.0.password'
- 'sensitive_labels.0.service_key'
- name: 'passwordWo'
type: String
description: |
An password for a notification channel. Channel types that support this field include: webhook_basicauth. For more info see [updating write-only attributes](/docs/providers/google/guides/using_write_only_attributes.html#updating-write-only-attributes)
write_only_legacy: true
required_with:
- 'passwordWoVersion'
conflicts:
- 'sensitive_labels.0.password'
- name: 'passwordWoVersion'
type: Integer
immutable: true
ignore_read: true
description: |
Triggers a new password_wo to be written. Increment this to update password_wo. For more info see [updating write-only attributes](/docs/providers/google/guides/using_write_only_attributes.html#updating-write-only-attributes)
required_with:
- 'sensitive_labels.0.password_wo'
- name: 'serviceKey'
type: String
description: |
An servicekey token for a notification channel. Channel types that support this field include: pagerduty
sensitive: true
conflicts:
- 'sensitive_labels.0.service_key_wo'
exactly_one_of:
- 'sensitive_labels.0.auth_token'
- 'sensitive_labels.0.password'
- 'sensitive_labels.0.service_key'
- name: 'serviceKeyWo'
type: String
description: |
An servicekey token for a notification channel. Channel types that support this field include: pagerduty. For more info see [updating write-only attributes](/docs/providers/google/guides/using_write_only_attributes.html#updating-write-only-attributes)
write_only_legacy: true
required_with:
- 'serviceKeyWoVersion'
conflicts:
- 'sensitive_labels.0.service_key'
- name: 'serviceKeyWoVersion'
type: Integer
immutable: true
ignore_read: true
description: |
Triggers a new service_key_wo to be written. Increment this to update service_key_wo. For more info see [updating write-only attributes](/docs/providers/google/guides/using_write_only_attributes.html#updating-write-only-attributes)
required_with:
- 'sensitive_labels.0.service_key_wo'
- name: 'name'
type: String
description: |
Expand Down
2 changes: 2 additions & 0 deletions mmv1/templates/terraform/decoders/monitoring_notification_channel.go.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ if labelmap, ok := res["labels"]; ok {
if _, apiOk := labels[sl]; apiOk {
if _, exists := d.GetOkExists("sensitive_labels.0." + sl); exists {
delete(labels, sl)
} else if _, existsWo := d.GetOkExists("sensitive_labels.0." + sl + "_wo"); existsWo {
delete(labels, sl)
} else {
labels[sl] = d.Get("labels." + sl)
}
Expand Down
3 changes: 3 additions & 0 deletions mmv1/templates/terraform/encoders/monitoring_notification_channel.go.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ for _, sl := range sensitiveLabels {
if auth, _ := d.GetOkExists("sensitive_labels.0." + sl); auth != "" {
labels[sl] = auth.(string)
}
if authWo, _ := d.GetOkExists("sensitive_labels.0." + sl + "_wo"); authWo != "" {
labels[sl] = authWo.(string)
}
}

obj["labels"] = labels
Expand Down
131 changes: 131 additions & 0 deletions ...hird_party/terraform/services/monitoring/resource_monitoring_notification_channel_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"testing"

"github.com/hashicorp/terraform-plugin-testing/helper/resource"
"github.com/hashicorp/terraform-plugin-testing/plancheck"
"github.com/hashicorp/terraform-provider-google/google/acctest"
)

Expand Down Expand Up @@ -139,6 +140,91 @@ func TestAccMonitoringNotificationChannel_updateSensitiveLabels(t *testing.T) {
})
}

func TestAccMonitoringNotificationChannel_updateSensitiveLabelsWriteOnly_slack(t *testing.T) {
// Slack auth_token required for test not to fail, skipping test till internal testing slack can be created
t.Skip()
t.Parallel()

acctest.VcrTest(t, resource.TestCase{
PreCheck: func() { acctest.AccTestPreCheck(t) },
ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t),
CheckDestroy: testAccCheckMonitoringNotificationChannelDestroyProducer(t),
Steps: []resource.TestStep{
{
Config: testAccMonitoringNotificationChannel_sensitiveLabelsWriteOnly_slack("token1", 1),
},
{
ResourceName: "google_monitoring_notification_channel.slack",
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{"labels.%", "labels.auth_token", "sensitive_labels"},
},
{
Config: testAccMonitoringNotificationChannel_sensitiveLabelsWriteOnly_slack("token2", 2),
ConfigPlanChecks: resource.ConfigPlanChecks{
PreApply: []plancheck.PlanCheck{
plancheck.ExpectResourceAction("google_monitoring_notification_channel.slack", plancheck.ResourceActionUpdate),
},
},
},
{
ResourceName: "google_monitoring_notification_channel.slack",
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{"labels.%", "labels.auth_token", "sensitive_labels"},
},
},
})
}

func TestAccMonitoringNotificationChannel_updateSensitiveLabelsWriteOnly(t *testing.T) {
t.Parallel()

acctest.VcrTest(t, resource.TestCase{
PreCheck: func() { acctest.AccTestPreCheck(t) },
ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t),
CheckDestroy: testAccCheckMonitoringNotificationChannelDestroyProducer(t),
Steps: []resource.TestStep{
{
Config: testAccMonitoringNotificationChannel_sensitiveLabelsWriteOnly("key1", "pass1", 1),
},
{
ResourceName: "google_monitoring_notification_channel.pagerduty",
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{"labels.%", "labels.service_key", "sensitive_labels"},
},
{
ResourceName: "google_monitoring_notification_channel.basicauth",
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{"labels.%", "labels.password", "sensitive_labels"},
},
{
Config: testAccMonitoringNotificationChannel_sensitiveLabelsWriteOnly("key2", "pass2", 2),
ConfigPlanChecks: resource.ConfigPlanChecks{
PreApply: []plancheck.PlanCheck{
plancheck.ExpectResourceAction("google_monitoring_notification_channel.pagerduty", plancheck.ResourceActionUpdate),
plancheck.ExpectResourceAction("google_monitoring_notification_channel.basicauth", plancheck.ResourceActionUpdate),
},
},
},
{
ResourceName: "google_monitoring_notification_channel.pagerduty",
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{"labels.%", "labels.service_key", "sensitive_labels"},
},
{
ResourceName: "google_monitoring_notification_channel.basicauth",
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{"labels.%", "labels.password", "sensitive_labels"},
},
},
})
}

func testAccMonitoringNotificationChannel_update(channel, labels, enabled string) string {
return fmt.Sprintf(`
resource "google_monitoring_notification_channel" "update" {
Expand Down Expand Up @@ -233,3 +319,48 @@ resource "google_monitoring_notification_channel" "pagerduty" {
}
`)
}

func testAccMonitoringNotificationChannel_sensitiveLabelsWriteOnly_slack(authToken string, version int) string {
return fmt.Sprintf(`
resource "google_monitoring_notification_channel" "slack" {
display_name = "TFTest Slack Channel"
type = "slack"
labels = {
"channel_name" = "#foobar"
}

sensitive_labels {
auth_token_wo = "%s"
auth_token_wo_version = %d
}
}
`, authToken, version)
}

func testAccMonitoringNotificationChannel_sensitiveLabelsWriteOnly(serviceKey, password string, version int) string {
return fmt.Sprintf(`
resource "google_monitoring_notification_channel" "basicauth" {
display_name = "TFTest Basicauth Channel"
type = "webhook_basicauth"
labels = {
"username" = "username"
"url" = "http://fakeurl.com"
}

sensitive_labels {
password_wo = "%s"
password_wo_version = %d
}
}

resource "google_monitoring_notification_channel" "pagerduty" {
display_name = "TFTest Pagerduty Channel"
type = "pagerduty"

sensitive_labels {
service_key_wo = "%s"
service_key_wo_version = %d
}
}
`, password, version, serviceKey, version)
}
Loading