Skip to content

Commit

Permalink
refactor: package terraform plan processing
Browse files Browse the repository at this point in the history 
part of a set of generic improvements and refactorings. the goal is to improve overall stability and to make it easier to expand and contribute new checks to the project.

this change moves terraform plan processing into own package.
  • Loading branch information
@roope-kar
roope-kar committed Dec 18, 2024
1 parent b8cf8b9 commit 1b6caa2
Show file tree
Hide file tree
Showing 8 changed files with 324 additions and 227 deletions.
Binary file modified build/checker
View file
Binary file not shown.
73 changes: 45 additions & 28 deletions checks.go
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package main

import (
"aiven/terraform/governance/compliance/checker/internal/terraform"
"slices"
)

Expand All @@ -10,15 +11,15 @@ type CheckResult struct {
}

func changeIsRequestedByOwner(
resourceChange ResourceChange,
requester *StateResource,
_ []*StateResource,
plan *Plan,
resourceChange terraform.ResourceChange,
requester *terraform.PriorStateResource,
_ []*terraform.PriorStateResource,
plan *terraform.Plan,
) CheckResult {
checkResult := CheckResult{ok: true, errors: []ResultError{}}

// If the owner is defined but it's a new group it's in the state post-apply so we have to use config to check it
if resourceChange.Change.AfterUnknown.OwnerUserGroupID {
if ownerAfterApply := resourceChange.Change.AfterUnknown.OwnerUserGroupID; ownerAfterApply != nil && *ownerAfterApply {
if !isUserGroupMemberInConfig(resourceChange, requester, plan) {
checkResult.ok = false
checkResult.errors = append(checkResult.errors,
Expand All @@ -31,19 +32,19 @@ func changeIsRequestedByOwner(
}

// When the resource is created, the requester must be a member of the owner group after the change
if slices.Contains(resourceChange.Change.Actions, "create") {
if slices.Contains(resourceChange.Change.Actions, terraform.CreateAction) {
checkResult.errors = append(checkResult.errors,
validateRequesterFromState(resourceChange.Address, resourceChange.Change.After, requester, plan)...)
}
// When the resource is updated, the requester must be a member of the owner group before and after the change
if slices.Contains(resourceChange.Change.Actions, "update") {
if slices.Contains(resourceChange.Change.Actions, terraform.UpdateAction) {
checkResult.errors = append(checkResult.errors,
validateRequesterFromState(resourceChange.Address, resourceChange.Change.Before, requester, plan)...)
checkResult.errors = append(checkResult.errors,
validateRequesterFromState(resourceChange.Address, resourceChange.Change.After, requester, plan)...)
}
// When the resource is deleted, the requester must be a member of the owner group before the change
if slices.Contains(resourceChange.Change.Actions, "delete") {
if slices.Contains(resourceChange.Change.Actions, terraform.DeleteAction) {
checkResult.errors = append(checkResult.errors,
validateRequesterFromState(resourceChange.Address, resourceChange.Change.Before, requester, plan)...)
}
Expand All @@ -55,15 +56,15 @@ func changeIsRequestedByOwner(
}

func changeIsApprovedByOwner(
resourceChange ResourceChange,
_ *StateResource,
approvers []*StateResource,
plan *Plan,
resourceChange terraform.ResourceChange,
_ *terraform.PriorStateResource,
approvers []*terraform.PriorStateResource,
plan *terraform.Plan,
) CheckResult {
checkResult := CheckResult{ok: true, errors: []ResultError{}}

// If the owner is defined but it's a new group it's in the state post-apply so we have to use config to check it
if resourceChange.Change.AfterUnknown.OwnerUserGroupID {
if ownerAfterApply := resourceChange.Change.AfterUnknown.OwnerUserGroupID; ownerAfterApply != nil && *ownerAfterApply {
foundApprover := false
for _, approver := range approvers {
if isUserGroupMemberInConfig(resourceChange, approver, plan) {
Expand All @@ -83,14 +84,14 @@ func changeIsApprovedByOwner(
}

// When the resource is created, the approvers must be a member of the owner group after the change
if slices.Contains(resourceChange.Change.Actions, "create") {
if slices.Contains(resourceChange.Change.Actions, terraform.CreateAction) {
checkResult.errors = append(
checkResult.errors,
validateApproversFromState(resourceChange.Address, resourceChange.Change.After, approvers, plan)...,
)
}

if slices.Contains(resourceChange.Change.Actions, "update") {
if slices.Contains(resourceChange.Change.Actions, terraform.UpdateAction) {
// updating owner requires approvals from both old and the new owner
// in other cases checking Change.After would be redundant
checkResult.errors = append(
Expand All @@ -104,7 +105,7 @@ func changeIsApprovedByOwner(
}

// When the resource is deleted, the approvers must be a member of the owner group before the change
if slices.Contains(resourceChange.Change.Actions, "delete") {
if slices.Contains(resourceChange.Change.Actions, terraform.DeleteAction) {
checkResult.errors = append(
checkResult.errors,
validateApproversFromState(resourceChange.Address, resourceChange.Change.Before, approvers, plan)...,
Expand All @@ -119,9 +120,9 @@ func changeIsApprovedByOwner(

func validateApproversFromState(
address string,
resource *ChangeResource,
approvers []*StateResource,
plan *Plan,
resource *terraform.ResourceChangeValues,
approvers []*terraform.PriorStateResource,
plan *terraform.Plan,
) []ResultError {
resultErrors := []ResultError{}

Expand Down Expand Up @@ -151,9 +152,9 @@ func validateApproversFromState(

func validateRequesterFromState(
address string,
resource *ChangeResource,
requester *StateResource,
plan *Plan,
resource *terraform.ResourceChangeValues,
requester *terraform.PriorStateResource,
plan *terraform.Plan,
) []ResultError {
resultErrors := []ResultError{}

Expand All @@ -178,18 +179,34 @@ func validateRequesterFromState(
return resultErrors
}

func newRequestError(address string, tag []Tag) ResultError {
func newRequestError(address string, tag *[]terraform.Tag) ResultError {
err := "requesting user is not a member of the owner group"
if tag != nil {
return ResultError{
Error: err,
Address: address,
Tags: *tag,
}
}
return ResultError{
Error: "requesting user is not a member of the owner group",
Error: err,
Address: address,
Tags: tag,
Tags: []terraform.Tag{},
}
}

func newApproveError(address string, tag []Tag) ResultError {
func newApproveError(address string, tag *[]terraform.Tag) ResultError {
err := "approval is required from a member of the owner group"
if tag != nil {
return ResultError{
Error: err,
Address: address,
Tags: *tag,
}
}
return ResultError{
Error: "approval is required from a member of the owner group",
Error: err,
Address: address,
Tags: tag,
Tags: []terraform.Tag{},
}
}
72 changes: 37 additions & 35 deletions checks_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,44 +3,46 @@ package main
import (
"testing"

"aiven/terraform/governance/compliance/checker/internal/terraform"

"github.com/stretchr/testify/assert"
)

func TestUnit_ValidateApproversFromStateSimpleCases(t *testing.T) {
tests := []struct {
name string
address string
resource *ChangeResource
approvers []*StateResource
plan *Plan
resource *terraform.ResourceChangeValues
approvers []*terraform.PriorStateResource
plan *terraform.Plan
expectedErrors int
}{
{
name: "Resource has no owner group ID",
address: "resource1",
resource: &ChangeResource{OwnerUserGroupID: nil},
approvers: []*StateResource{{}},
resource: &terraform.ResourceChangeValues{OwnerUserGroupID: nil},
approvers: []*terraform.PriorStateResource{{}},
expectedErrors: 0,
},
{
name: "Resource owner group ID is empty",
address: "resource2",
resource: &ChangeResource{OwnerUserGroupID: stringPtr("")},
approvers: []*StateResource{{}},
resource: &terraform.ResourceChangeValues{OwnerUserGroupID: stringPtr("")},
approvers: []*terraform.PriorStateResource{{}},
expectedErrors: 0,
},
{
name: "Resource is nil",
address: "resource3",
resource: nil,
approvers: []*StateResource{{}},
approvers: []*terraform.PriorStateResource{{}},
expectedErrors: 0,
},
{
name: "No approvers",
address: "resource4",
resource: &ChangeResource{OwnerUserGroupID: stringPtr("owner-group")},
approvers: []*StateResource{},
resource: &terraform.ResourceChangeValues{OwnerUserGroupID: stringPtr("owner-group")},
approvers: []*terraform.PriorStateResource{},
expectedErrors: 1,
},
}
Expand All @@ -58,29 +60,29 @@ func TestUnit_ValidateRequesterFromStateSimpleCases(t *testing.T) {
tests := []struct {
name string
address string
resource *ChangeResource
requester *StateResource
plan *Plan
resource *terraform.ResourceChangeValues
requester *terraform.PriorStateResource
plan *terraform.Plan
expectedErrors int
}{
{
name: "Resource has no owner group ID",
address: "resource3",
resource: &ChangeResource{OwnerUserGroupID: nil},
requester: &StateResource{},
resource: &terraform.ResourceChangeValues{OwnerUserGroupID: nil},
requester: &terraform.PriorStateResource{},
expectedErrors: 0,
},
{
name: "Resource owner group ID is empty",
address: "resource4",
resource: &ChangeResource{OwnerUserGroupID: stringPtr("")},
requester: &StateResource{},
resource: &terraform.ResourceChangeValues{OwnerUserGroupID: stringPtr("")},
requester: &terraform.PriorStateResource{},
expectedErrors: 0,
},
{
name: "Requester is nil",
address: "resource5",
resource: &ChangeResource{OwnerUserGroupID: stringPtr("owner-group")},
resource: &terraform.ResourceChangeValues{OwnerUserGroupID: stringPtr("owner-group")},

requester: nil,
expectedErrors: 1,
Expand All @@ -89,7 +91,7 @@ func TestUnit_ValidateRequesterFromStateSimpleCases(t *testing.T) {
name: "Resource is nil",
address: "resource6",
resource: nil,
requester: &StateResource{},
requester: &terraform.PriorStateResource{},
expectedErrors: 0,
},
}
Expand All @@ -108,44 +110,44 @@ func TestUnit_NewRequestError(t *testing.T) {
tests := []struct {
name string
address string
tag []Tag
tag []terraform.Tag
expected ResultError
}{
{
name: "Single tag",
address: "resource1",
tag: []Tag{{Key: "env", Value: "prod"}},
tag: []terraform.Tag{{Key: "env", Value: "prod"}},
expected: ResultError{
Error: "requesting user is not a member of the owner group",
Address: "resource1",
Tags: []Tag{{Key: "env", Value: "prod"}},
Tags: []terraform.Tag{{Key: "env", Value: "prod"}},
},
},
{
name: "Multiple tags",
address: "resource2",
tag: []Tag{{Key: "env", Value: "prod"}, {Key: "team", Value: "devops"}},
tag: []terraform.Tag{{Key: "env", Value: "prod"}, {Key: "team", Value: "devops"}},
expected: ResultError{
Error: "requesting user is not a member of the owner group",
Address: "resource2",
Tags: []Tag{{Key: "env", Value: "prod"}, {Key: "team", Value: "devops"}},
Tags: []terraform.Tag{{Key: "env", Value: "prod"}, {Key: "team", Value: "devops"}},
},
},
{
name: "No tags",
address: "resource3",
tag: []Tag{},
tag: []terraform.Tag{},
expected: ResultError{
Error: "requesting user is not a member of the owner group",
Address: "resource3",
Tags: []Tag{},
Tags: []terraform.Tag{},
},
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
result := newRequestError(tt.address, tt.tag)
result := newRequestError(tt.address, &tt.tag)
if !assert.ObjectsAreEqual(tt.expected, result) {
t.Errorf("expected %v, got %v", tt.expected, result)
}
Expand All @@ -157,44 +159,44 @@ func TestUnit_NewApproveError(t *testing.T) {
tests := []struct {
name string
address string
tag []Tag
tag []terraform.Tag
expected ResultError
}{
{
name: "Single tag",
address: "resource1",
tag: []Tag{{Key: "env", Value: "prod"}},
tag: []terraform.Tag{{Key: "env", Value: "prod"}},
expected: ResultError{
Error: "approval is required from a member of the owner group",
Address: "resource1",
Tags: []Tag{{Key: "env", Value: "prod"}},
Tags: []terraform.Tag{{Key: "env", Value: "prod"}},
},
},
{
name: "Multiple tags",
address: "resource2",
tag: []Tag{{Key: "env", Value: "prod"}, {Key: "team", Value: "devops"}},
tag: []terraform.Tag{{Key: "env", Value: "prod"}, {Key: "team", Value: "devops"}},
expected: ResultError{
Error: "approval is required from a member of the owner group",
Address: "resource2",
Tags: []Tag{{Key: "env", Value: "prod"}, {Key: "team", Value: "devops"}},
Tags: []terraform.Tag{{Key: "env", Value: "prod"}, {Key: "team", Value: "devops"}},
},
},
{
name: "No tags",
address: "resource3",
tag: []Tag{},
tag: []terraform.Tag{},
expected: ResultError{
Error: "approval is required from a member of the owner group",
Address: "resource3",
Tags: []Tag{},
Tags: []terraform.Tag{},
},
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
result := newApproveError(tt.address, tt.tag)
result := newApproveError(tt.address, &tt.tag)
if !assert.ObjectsAreEqual(tt.expected, result) {
t.Errorf("expected %v, got %v", tt.expected, result)
}
Expand Down
Loading

0 comments on commit 1b6caa2

Please sign in to comment.