Skip to content

Commit

Permalink
fix: do not create policy role assignment is parameter has no value a…
Browse files Browse the repository at this point in the history
…nd is optional (#138)
  • Loading branch information
matt-FFFFFF authored Jul 10, 2024
1 parent 6b3dfb5 commit 06a0796
Show file tree
Hide file tree
Showing 13 changed files with 101 additions and 32 deletions.
17 changes: 17 additions & 0 deletions alzlib.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"net/url"
"os"
"path/filepath"
"slices"
"strings"
"sync"

Expand Down Expand Up @@ -173,6 +174,7 @@ func (az *AlzLib) PolicyAssignments() []string {
for k := range az.policyAssignments {
result = append(result, k)
}
slices.Sort(result)
return result
}

Expand All @@ -184,6 +186,7 @@ func (az *AlzLib) PolicyDefinitions() []string {
for k := range az.policyDefinitions {
result = append(result, k)
}
slices.Sort(result)
return result
}

Expand All @@ -195,6 +198,7 @@ func (az *AlzLib) PolicySetDefinitions() []string {
for k := range az.policySetDefinitions {
result = append(result, k)
}
slices.Sort(result)
return result
}

Expand All @@ -206,6 +210,7 @@ func (az *AlzLib) RoleDefinitions() []string {
for k := range az.roleDefinitions {
result = append(result, k)
}
slices.Sort(result)
return result
}

Expand All @@ -217,6 +222,7 @@ func (az *AlzLib) Archetypes() []string {
for k := range az.archetypes {
result = append(result, k)
}
slices.Sort(result)
return result
}

Expand All @@ -241,6 +247,17 @@ func (az *AlzLib) Architectures() []string {
return result
}

func (az *AlzLib) PolicyDefaultValues() []string {
az.mu.RLock()
defer az.mu.RUnlock()
result := make([]string, 0, len(az.defaultPolicyAssignmentValues))
for k := range az.defaultPolicyAssignmentValues {
result = append(result, k)
}
slices.Sort(result)
return result
}

// Architecture returns the requested architecture.
func (az *AlzLib) Architecture(name string) (*Architecture, error) {
az.mu.RLock()
Expand Down
14 changes: 14 additions & 0 deletions alzlib_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armpolicy"
mapset "github.com/deckarep/golang-set/v2"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestNewAlzLibOptions(t *testing.T) {
Expand Down Expand Up @@ -454,3 +455,16 @@ func TestAddDefaultPolicyValues(t *testing.T) {
err = az.addDefaultPolicyAssignmentValues(res)
assert.ErrorContains(t, err, "assignment `assignment1` and parameter `param1` already exists in defaults")
}

func TestInitSimple(t *testing.T) {
az := NewAlzLib(nil)
ctx := context.Background()
dirfs := os.DirFS("./testdata/simple")
require.NoError(t, az.Init(ctx, dirfs))
assert.Equal(t, []string{"empty", "simple", "simpleoverride"}, az.Archetypes())
assert.Equal(t, []string{"test-policy-definition"}, az.PolicyDefinitions())
assert.Equal(t, []string{"test-policy-set-definition"}, az.PolicySetDefinitions())
assert.Equal(t, []string{"test-role-definition"}, az.RoleDefinitions())
assert.Equal(t, []string{"override-policy-assignment", "test-policy-assignment"}, az.PolicyAssignments())
assert.Equal(t, []string{"test"}, az.PolicyDefaultValues())
}
14 changes: 14 additions & 0 deletions assets/policyDefinition.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,20 @@ func (pd *PolicyDefinition) AssignPermissionsParameterNames() ([]string, error)
return names, nil
}

func (pd *PolicyDefinition) ParameterIsOptional(name string) (bool, error) {
if pd == nil || pd.Properties == nil || pd.Properties.Parameters == nil {
return false, errors.New("PolicyDefinition.ParameterIsOptional: policy definition is nil, missing properties or parameters")
}
param, ok := pd.Properties.Parameters[name]
if !ok {
return false, fmt.Errorf("PolicyDefinition.ParameterIsOptional: parameter %s not found in policy definition", name)
}
if param.DefaultValue == nil {
return false, nil
}
return true, nil
}

func (pd *PolicyDefinition) Parameter(name string) *armpolicy.ParameterDefinitionsValue {
if pd == nil || pd.Properties == nil || pd.Properties.Parameters == nil {
return nil
Expand Down
11 changes: 9 additions & 2 deletions deployment/managementgroup.go
Original file line number Diff line number Diff line change
Expand Up @@ -201,11 +201,18 @@ func (mg *HierarchyManagementGroup) generatePolicyAssignmentAdditionalRoleAssign
return fmt.Errorf("ManagementGroup.GeneratePolicyAssignmentAdditionalRoleAssignments: error getting assign permissions parameter names for policy definition `%s`: %w", *pd.Name, err)
}
for _, paramName := range assignPermissionParams {

paParamVal, err := pa.ParameterValueAsString(paramName)
paramIsOptional, err := pd.ParameterIsOptional(paramName)
if err != nil {
return fmt.Errorf("ManagementGroup.GeneratePolicyAssignmentAdditionalRoleAssignments: error getting parameter %s optional status for policy definition `%s`: %w", paramName, *pd.Name, err)
}
paParamVal, err := pa.ParameterValueAsString(paramName)
if err != nil && !paramIsOptional {
return fmt.Errorf("ManagementGroup.GeneratePolicyAssignmentAdditionalRoleAssignments: error getting parameter value for parameter `%s` in policy assignment `%s`: %w", paramName, paName, err)
}
// We should assign permissions but the parameter os optional and doesn't have a value in the assignment, so skip.
if err != nil && paramIsOptional {
continue
}
if paParamVal == "" {
continue
}
Expand Down
12 changes: 9 additions & 3 deletions integrationtest/alzlib_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,11 @@ import (
"github.com/stretchr/testify/require"
)

const (
alzLibraryTag = "2024.07.02"
alzLibraryMember = "platform/alz"
)

func TestNewAlzLibOptionsError(t *testing.T) {
az := new(alzlib.AlzLib)
ctx, cancel := context.WithCancel(context.Background())
Expand All @@ -33,11 +38,12 @@ func TestInitMultiLib(t *testing.T) {
dirfs := os.DirFS("../testdata/simple")
err = az.Init(ctx, remoteLib, dirfs)
require.NoError(t, err)
assert.Equal(t, 12, len(az.Archetypes()))
assert.Equal(t, 13, len(az.Archetypes()))
// Test root archetype has been overridden
arch, _ := az.Archetype("root")
assert.Equal(t, 1, arch.PolicyDefinitions.Cardinality())
arch, _ = az.Archetype("simpleo")
assert.Equal(t, 158, arch.PolicyDefinitions.Cardinality())
arch, err = az.Archetype("simpleoverride")
require.NoError(t, err)
assert.Equal(t, 1, arch.PolicyDefinitions.Cardinality())
assert.Equal(t, 1, arch.PolicyAssignments.Cardinality())
}
5 changes: 0 additions & 5 deletions integrationtest/examples_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,6 @@ import (
"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armpolicy"
)

const (
alzLibraryTag = "2024.07.01"
alzLibraryMember = "platform/alz"
)

// Example_deploymentNewHierarchy tests the ALZ reference architecture creation in full.
func Example_deploymentNewHierarchy() {
az := alzlib.NewAlzLib(nil)
Expand Down
15 changes: 0 additions & 15 deletions testdata/simple/simple.alz_archetype_definition.json

This file was deleted.

10 changes: 10 additions & 0 deletions testdata/simple/simple.alz_archetype_definition.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
---
name: simple
policy_assignments:
- test-policy-assignment
policy_definitions:
- test-policy-definition
policy_set_definitions:
- test-policy-set-definition
role_definitions:
- test-role-definition
14 changes: 14 additions & 0 deletions testdata/simple/simple.alz_architecture_definition.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
management_groups:
- archetypes:
- simple
display_name: simple
exists: false
id: simple
parent_id: null
- archetypes:
- simpleoverride
display_name: simple overide
exists: false
id: simpleoverride
parent_id: null
name: simple
6 changes: 0 additions & 6 deletions testdata/simple/simpleo.alz_archetype_override.json

This file was deleted.

7 changes: 7 additions & 0 deletions testdata/simple/simpleo.alz_archetype_override.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
base_archetype: simple
name: simpleoverride
policy_assignments_to_add:
- override-policy-assignment
policy_assignments_to_remove:
- test-policy-assignment
2 changes: 1 addition & 1 deletion testdata/simple/test.alz_policy_assignment.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
],
"notScopes": [],
"parameters": {},
"policyDefinitionId": "/providers/Microsoft.Authorization/policySetDefinitions/test-policy-set-definition",
"policyDefinitionId": "/providers/Microsoft.Authorization/policySetDefinitions/test-policy-definition",
"scope": "/providers/Microsoft.Management/managementGroups/PLACEHOLDER"
},
"type": "Microsoft.Authorization/policyAssignments"
Expand Down
6 changes: 6 additions & 0 deletions testdata/simple/test.alz_policy_default_value.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
name: test
policy_assignments:
- policy_assignment_name: test-policy-assignment
parameter_names:
- effect

0 comments on commit 06a0796

Please sign in to comment.