Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 2 additions & 1 deletion pipelines/types/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`

Expand All @@ -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)
}
Expand Down
3 changes: 3 additions & 0 deletions pipelines/types/pipeline.schema.v1.json
Original file line number Diff line number Diff line change
Expand Up @@ -796,6 +796,9 @@
"zoneRedundancy": {
"$ref": "#/definitions/value"
},
"publicNetworkAccess": {
"$ref": "#/definitions/value"
},
"crossTenantSecurityGroup": {
"$ref": "#/definitions/value"
},
Expand Down
5 changes: 4 additions & 1 deletion tools/grafanactl/cmd/manage/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 != "" {
Expand Down Expand Up @@ -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),
Expand All @@ -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
Expand Down
13 changes: 10 additions & 3 deletions tools/grafanactl/cmd/manage/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ type RawReconcileOptions struct {
SKU string
MajorVersion string
ZoneRedundancy string
PublicNetworkAccess string
CrossTenantSecurityGroup string
}

Expand All @@ -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",

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Defaulting to "Disabled" provides a secure-by-default design but risks locking down all environments if this change isn't paired with the new ARO-HCP changes in a timely manner.

Defaulting to "Enabled" ensures backwards-compatibility if these were merged early but breaks the secure-by-default pattern.

My vote is for secure by default as I think the risk is low; however, I'm open to feedback.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Going to switch this to backwards compatible (enabled by default)

}
}

Expand All @@ -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
Expand All @@ -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,
Expand Down
90 changes: 90 additions & 0 deletions tools/grafanactl/cmd/manage/options_test.go
Original file line number Diff line number Diff line change
@@ -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

Check failure on line 39 in tools/grafanactl/cmd/manage/options_test.go

View workflow job for this annotation

GitHub Actions / verify

File is not properly formatted (gci)
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)
}
})
}
}
Loading