From 1ce7f719db261c12711b3484b4191ea909301b52 Mon Sep 17 00:00:00 2001 From: Cliff Schomburg Date: Thu, 30 Jul 2026 16:31:25 -0700 Subject: [PATCH 1/2] feat(grafanactl): add PublicNetworkAccess support for Grafana instances Add the ability to control the PublicNetworkAccess property on Azure Managed Grafana instances. This enables restricting Grafana to MSFT Corp VPN by setting PublicNetworkAccess to Disabled. The default is Disabled (secure-by-default), matching the Azure SDK enum values of Enabled/Disabled. Environments that need public access (e.g. dev) can explicitly override to Enabled via config. Jira: ARO-28693 Co-Authored-By: Claude Opus 4.6 (1M context) --- pipelines/types/common.go | 3 +- pipelines/types/pipeline.schema.v1.json | 3 + tools/grafanactl/cmd/manage/cmd.go | 5 +- tools/grafanactl/cmd/manage/options.go | 13 ++- tools/grafanactl/cmd/manage/options_test.go | 90 +++++++++++++++++++++ 5 files changed, 109 insertions(+), 5 deletions(-) create mode 100644 tools/grafanactl/cmd/manage/options_test.go diff --git a/pipelines/types/common.go b/pipelines/types/common.go index 89165b1a..790beaff 100644 --- a/pipelines/types/common.go +++ b/pipelines/types/common.go @@ -810,6 +810,7 @@ type GrafanaManageStep struct { SKU Value `json:"sku,omitempty"` MajorVersion Value `json:"majorVersion,omitempty"` ZoneRedundancy Value `json:"zoneRedundancy,omitempty"` + PublicNetworkAccess Value `json:"publicNetworkAccess,omitempty"` CrossTenantSecurityGroup Value `json:"crossTenantSecurityGroup,omitempty"` Timeout string `json:"timeout,omitempty"` @@ -823,7 +824,7 @@ func (s *GrafanaManageStep) Description() string { func (s *GrafanaManageStep) RequiredInputs() []StepDependency { var deps []StepDependency - for _, val := range []Value{s.GrafanaName, s.Location, s.SKU, s.MajorVersion, s.ZoneRedundancy, s.CrossTenantSecurityGroup} { + for _, val := range []Value{s.GrafanaName, s.Location, s.SKU, s.MajorVersion, s.ZoneRedundancy, s.PublicNetworkAccess, s.CrossTenantSecurityGroup} { if val.Input != nil { deps = append(deps, val.Input.StepDependency) } diff --git a/pipelines/types/pipeline.schema.v1.json b/pipelines/types/pipeline.schema.v1.json index 1e1612c3..baefac23 100644 --- a/pipelines/types/pipeline.schema.v1.json +++ b/pipelines/types/pipeline.schema.v1.json @@ -796,6 +796,9 @@ "zoneRedundancy": { "$ref": "#/definitions/value" }, + "publicNetworkAccess": { + "$ref": "#/definitions/value" + }, "crossTenantSecurityGroup": { "$ref": "#/definitions/value" }, diff --git a/tools/grafanactl/cmd/manage/cmd.go b/tools/grafanactl/cmd/manage/cmd.go index 6c086811..87e42034 100644 --- a/tools/grafanactl/cmd/manage/cmd.go +++ b/tools/grafanactl/cmd/manage/cmd.go @@ -86,6 +86,7 @@ func (o *CompletedReconcileOptions) Run(ctx context.Context) error { logger.Info("reconcile command executed", "dry-run", o.DryRun) zoneRedundancy := armdashboard.ZoneRedundancy(o.ZoneRedundancy) + publicNetworkAccess := armdashboard.PublicNetworkAccess(o.PublicNetworkAccess) tags := map[string]*string{} if o.CrossTenantSecurityGroup != "" { @@ -133,7 +134,8 @@ func (o *CompletedReconcileOptions) Run(ctx context.Context) error { }, Tags: tags, Properties: &armdashboard.ManagedGrafanaProperties{ - ZoneRedundancy: &zoneRedundancy, + PublicNetworkAccess: &publicNetworkAccess, + ZoneRedundancy: &zoneRedundancy, GrafanaConfigurations: &armdashboard.GrafanaConfigurations{ Users: &armdashboard.Users{ ViewersCanEdit: to.Ptr(true), @@ -153,6 +155,7 @@ func (o *CompletedReconcileOptions) Run(ctx context.Context) error { "location", o.Location, "major-version", o.MajorVersion, "zone-redundancy", o.ZoneRedundancy, + "public-network-access", o.PublicNetworkAccess, "integrations", workspaceIDs.Len(), ) return nil diff --git a/tools/grafanactl/cmd/manage/options.go b/tools/grafanactl/cmd/manage/options.go index c5017917..c9cfd730 100644 --- a/tools/grafanactl/cmd/manage/options.go +++ b/tools/grafanactl/cmd/manage/options.go @@ -32,6 +32,7 @@ type RawReconcileOptions struct { SKU string MajorVersion string ZoneRedundancy string + PublicNetworkAccess string CrossTenantSecurityGroup string } @@ -56,9 +57,10 @@ type CompletedReconcileOptions struct { // DefaultReconcileOptions returns a new RawReconcileOptions with default values func DefaultReconcileOptions() *RawReconcileOptions { return &RawReconcileOptions{ - BaseOptions: base.DefaultBaseOptions(), - SKU: "Standard", - ZoneRedundancy: "Disabled", + BaseOptions: base.DefaultBaseOptions(), + SKU: "Standard", + ZoneRedundancy: "Disabled", + PublicNetworkAccess: "Disabled", } } @@ -73,6 +75,7 @@ func BindReconcileOptions(opts *RawReconcileOptions, cmd *cobra.Command) error { flags.StringVar(&opts.SKU, "sku", opts.SKU, "Grafana SKU name (e.g. Standard)") flags.StringVar(&opts.MajorVersion, "major-version", opts.MajorVersion, "Grafana major version (e.g. 11)") flags.StringVar(&opts.ZoneRedundancy, "zone-redundancy", opts.ZoneRedundancy, "Zone redundancy mode: Enabled or Disabled") + flags.StringVar(&opts.PublicNetworkAccess, "public-network-access", opts.PublicNetworkAccess, "Public network access mode: Enabled or Disabled") flags.StringVar(&opts.CrossTenantSecurityGroup, "cross-tenant-security-group", opts.CrossTenantSecurityGroup, "Cross-tenant security group (format: GroupObjectId;TenantId)") return nil @@ -97,6 +100,10 @@ func (o *RawReconcileOptions) Validate(ctx context.Context) (*ValidatedReconcile return nil, fmt.Errorf("--zone-redundancy must be 'Enabled' or 'Disabled', got: %s", o.ZoneRedundancy) } + if o.PublicNetworkAccess != "Enabled" && o.PublicNetworkAccess != "Disabled" { + return nil, fmt.Errorf("--public-network-access must be 'Enabled' or 'Disabled', got: %s", o.PublicNetworkAccess) + } + return &ValidatedReconcileOptions{ validatedReconcileOptions: &validatedReconcileOptions{ RawReconcileOptions: o, diff --git a/tools/grafanactl/cmd/manage/options_test.go b/tools/grafanactl/cmd/manage/options_test.go new file mode 100644 index 00000000..474d3cc5 --- /dev/null +++ b/tools/grafanactl/cmd/manage/options_test.go @@ -0,0 +1,90 @@ +// Copyright 2026 Microsoft Corporation +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package manage + +import ( + "context" + "strings" + "testing" +) + +func TestDefaultReconcileOptions(t *testing.T) { + opts := DefaultReconcileOptions() + + if opts.SKU != "Standard" { + t.Fatalf("expected default SKU 'Standard', got %q", opts.SKU) + } + if opts.ZoneRedundancy != "Disabled" { + t.Fatalf("expected default ZoneRedundancy 'Disabled', got %q", opts.ZoneRedundancy) + } + if opts.PublicNetworkAccess != "Disabled" { + t.Fatalf("expected default PublicNetworkAccess 'Disabled', got %q", opts.PublicNetworkAccess) + } +} + +func TestValidatePublicNetworkAccess(t *testing.T) { + for _, tc := range []struct { + name string + publicNetworkAccess string + wantErrSub string + }{ + { + name: "Enabled is valid", + publicNetworkAccess: "Enabled", + }, + { + name: "Disabled is valid", + publicNetworkAccess: "Disabled", + }, + { + name: "empty string is rejected", + publicNetworkAccess: "", + wantErrSub: "--public-network-access must be", + }, + { + name: "invalid value is rejected", + publicNetworkAccess: "Invalid", + wantErrSub: "--public-network-access must be", + }, + { + name: "lowercase is rejected", + publicNetworkAccess: "enabled", + wantErrSub: "--public-network-access must be", + }, + } { + t.Run(tc.name, func(t *testing.T) { + opts := DefaultReconcileOptions() + opts.Location = "eastus" + opts.GrafanaName = "test-grafana" + opts.SubscriptionID = "00000000-0000-0000-0000-000000000000" + opts.ResourceGroup = "test-rg" + opts.PublicNetworkAccess = tc.publicNetworkAccess + + _, err := opts.Validate(context.Background()) + if tc.wantErrSub != "" { + if err == nil { + t.Fatalf("expected error containing %q, got nil", tc.wantErrSub) + } + if !strings.Contains(err.Error(), tc.wantErrSub) { + t.Fatalf("expected error containing %q, got %q", tc.wantErrSub, err.Error()) + } + return + } + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + }) + } +} From e83feef3aeadf8f8c9567cab2fe59fa58d5d47c0 Mon Sep 17 00:00:00 2001 From: Cliff Schomburg Date: Mon, 3 Aug 2026 06:50:05 -0700 Subject: [PATCH 2/2] fix: default PublicNetworkAccess to Enabled for backward compatibility Avoids accidentally disabling public access on all environments if the ARO-Tools dependency is bumped without the corresponding config wiring in ARO-HCP. The restriction to Disabled is enforced at the config layer in ARO-HCP instead. Co-Authored-By: Claude Opus 4.6 (1M context) --- tools/grafanactl/cmd/manage/options.go | 2 +- tools/grafanactl/cmd/manage/options_test.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tools/grafanactl/cmd/manage/options.go b/tools/grafanactl/cmd/manage/options.go index c9cfd730..0496bc61 100644 --- a/tools/grafanactl/cmd/manage/options.go +++ b/tools/grafanactl/cmd/manage/options.go @@ -60,7 +60,7 @@ func DefaultReconcileOptions() *RawReconcileOptions { BaseOptions: base.DefaultBaseOptions(), SKU: "Standard", ZoneRedundancy: "Disabled", - PublicNetworkAccess: "Disabled", + PublicNetworkAccess: "Enabled", } } diff --git a/tools/grafanactl/cmd/manage/options_test.go b/tools/grafanactl/cmd/manage/options_test.go index 474d3cc5..12a95c1c 100644 --- a/tools/grafanactl/cmd/manage/options_test.go +++ b/tools/grafanactl/cmd/manage/options_test.go @@ -29,8 +29,8 @@ func TestDefaultReconcileOptions(t *testing.T) { if opts.ZoneRedundancy != "Disabled" { t.Fatalf("expected default ZoneRedundancy 'Disabled', got %q", opts.ZoneRedundancy) } - if opts.PublicNetworkAccess != "Disabled" { - t.Fatalf("expected default PublicNetworkAccess 'Disabled', got %q", opts.PublicNetworkAccess) + if opts.PublicNetworkAccess != "Enabled" { + t.Fatalf("expected default PublicNetworkAccess 'Enabled', got %q", opts.PublicNetworkAccess) } }