Skip to content
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.

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

OCPBUGS-19757: bump controller iam policy to v2.4.7 #114

Merged
merged 3 commits into from
Oct 27, 2023
Merged
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
24 changes: 21 additions & 3 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -75,12 +75,16 @@ IAMCTL_OUTPUT_DIR ?= ./pkg/controllers/awsloadbalancercontroller
# Generated file name.
IAMCTL_OUTPUT_FILE ?= iam_policy.go

IAMCTL_OUTPUT_MINIFY_FILE ?= iam_policy_minify.go

# Go Package of the generated file.
IAMCTL_GO_PACKAGE ?= awsloadbalancercontroller

# File name of the generated CredentialsRequest CR.
IAMCTL_OUTPUT_CR_FILE ?= ./hack/controller/controller-credentials-request.yaml

IAMCTL_OUTPUT_MINIFY_CR_FILE ?= ./hack/controller/controller-credentials-request-minify.yaml

# Built go binary path.
IAMCTL_BINARY ?= ./bin/iamctl

Expand Down Expand Up @@ -127,13 +131,27 @@ vet: ## Run go vet against code.

.PHONY: iamctl-gen
iamctl-gen: iamctl-build iam-gen
$(IAMCTL_BINARY) -i $(IAMCTL_ASSETS_DIR)/iam-policy.json -o $(IAMCTL_OUTPUT_DIR)/$(IAMCTL_OUTPUT_FILE) -p $(IAMCTL_GO_PACKAGE) -c $(IAMCTL_OUTPUT_CR_FILE)
go fmt -mod=vendor $(IAMCTL_OUTPUT_DIR)/$(IAMCTL_OUTPUT_FILE)
go vet -mod=vendor $(IAMCTL_OUTPUT_DIR)/$(IAMCTL_OUTPUT_FILE)
# generate controller's IAM policy without minify.
@# This policy is for STS clusters as it's turned into a role policy which is limited to 10240 by AWS.
Copy link

Choose a reason for hiding this comment

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

Okay, so if I understand this correctly, you could minify for STS clusters as well, but by not minifying, you help expose issues with the non-minified IAM Policy, of which QE uses to install clusters and is documented Option 2. Using the AWS CLI in install.md?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, exactly.

$(IAMCTL_BINARY) -i $(IAMCTL_ASSETS_DIR)/iam-policy.json -o $(IAMCTL_OUTPUT_DIR)/$(IAMCTL_OUTPUT_FILE) -p $(IAMCTL_GO_PACKAGE) -c $(IAMCTL_OUTPUT_CR_FILE) -n -s

# generate controller's IAM policy with minify.
@# This policy is for non STS clusters as it's turned into an inline policy which is limited to 2048 by AWS.
$(IAMCTL_BINARY) -i $(IAMCTL_ASSETS_DIR)/iam-policy.json -o $(IAMCTL_OUTPUT_DIR)/$(IAMCTL_OUTPUT_MINIFY_FILE) -p $(IAMCTL_GO_PACKAGE) -f GetIAMPolicyMinify -c $(IAMCTL_OUTPUT_MINIFY_CR_FILE)

go fmt -mod=vendor $(IAMCTL_OUTPUT_DIR)/$(IAMCTL_OUTPUT_FILE) $(IAMCTL_OUTPUT_DIR)/$(IAMCTL_OUTPUT_MINIFY_FILE)
go vet -mod=vendor $(IAMCTL_OUTPUT_DIR)/$(IAMCTL_OUTPUT_FILE) $(IAMCTL_OUTPUT_DIR)/$(IAMCTL_OUTPUT_MINIFY_FILE)

# generate operator's IAM policy.
@# The operator's policy is small enough to fit into both limits: inline and role.
$(IAMCTL_BINARY) -i $(IAMCTL_ASSETS_DIR)/operator-iam-policy.json -o ./pkg/operator/$(IAMCTL_OUTPUT_FILE) -p operator -n
go fmt -mod=vendor ./pkg/operator/$(IAMCTL_OUTPUT_FILE)
go vet -mod=vendor ./pkg/operator/$(IAMCTL_OUTPUT_FILE)

# The operator's CredentialsRequest is the source of truth for the operator's IAM policy.
# It's required to generate IAM role for STS clusters using ccoctl (docs/prerequisites.md#option-1-using-ccoctl).
# The below rule generates a corresponding AWS IAM policy JSON which can be used in AWS CLI commands (docs/prerequisites.md#option-2-using-the-aws-cli).
# The operator's IAM policy as go code is generated from the JSON policy and used in the operator to self provision credentials at startup.
.PHONY: iam-gen
iam-gen:
./hack/generate-iam-from-credrequest.sh ./hack/operator-credentials-request.yaml ./hack/operator-permission-policy.json
Expand Down
22 changes: 22 additions & 0 deletions assets/iam-policy.json
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,28 @@
"arn:aws:elasticloadbalancing:*:*:listener-rule/app/*/*/*"
]
},
{
"Effect": "Allow",
"Action": [
"elasticloadbalancing:AddTags"
],
"Resource": [
"arn:aws:elasticloadbalancing:*:*:targetgroup/*/*",
"arn:aws:elasticloadbalancing:*:*:loadbalancer/net/*/*",
"arn:aws:elasticloadbalancing:*:*:loadbalancer/app/*/*"
],
"Condition": {
"StringEquals": {
"elasticloadbalancing:CreateAction": [
"CreateTargetGroup",
"CreateLoadBalancer"
]
},
"Null": {
"aws:RequestTag/elbv2.k8s.aws/cluster": "false"
}
}
},
{
"Effect": "Allow",
"Action": [
Expand Down
54 changes: 38 additions & 16 deletions cmd/iamctl/const.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,30 +6,43 @@ package {{.Package}}

import cco "github.com/openshift/cloud-credential-operator/pkg/apis/cloudcredential/v1"

{{- with .Definition }}
type IAMPolicy struct {
Version string
Statement []cco.StatementEntry
}
{{- end }}

func GetIAMPolicy() IAMPolicy {
func {{ .Function }}() IAMPolicy {
return IAMPolicy{
Statement: []cco.StatementEntry{
{{range .Statement -}}
{
Effect: {{.Effect|printf "%q"}},
Resource: {{range .Resource}}{{printf "%q" .}}{{end}},
PolicyCondition: cco.IAMPolicyCondition{},
{{- range .Statement }}
{
Effect: "{{ .Effect }}",
Resource: {{ range .Resource }}"{{ . }}"{{ end }},
PolicyCondition: cco.IAMPolicyCondition{
{{- with .Condition }}
{{- range $key, $value := . }}
"{{ $key }}": cco.IAMPolicyConditionKeyValue{
{{- range $innerKey, $innerValue := $value }}
"{{ $innerKey }}": {{ stringOrSlice $innerValue false }},
{{- end }}
},
{{- end }}
{{- end }}
},
Action: []string{
{{range $index, $element := .Action -}}
{{.|printf "%q"}},{{printf "\n"}}
{{- end}}
{{- range .Action }}
"{{ . }}",
{{- end }}
},
},
{{end}}
{{- end }}
},
}
}
`

credentialsRequestTemplate = `apiVersion: cloudcredential.openshift.io/v1
kind: CredentialsRequest
metadata:
Expand All @@ -40,14 +53,23 @@ spec:
apiVersion: cloudcredential.openshift.io/v1
kind: AWSProviderSpec
statementEntries:
{{range .Statement -}}
{{- range .Statement }}
- action:
{{range $index, $element := .Action}}- {{$element}}
{{end -}}

effect: {{.Effect}}
{{- range .Action }}
- {{ . }}
{{- end }}
effect: {{ .Effect }}
resource: {{range .Resource}}{{printf "%q" .}}{{end}}
{{- end}}
{{- with .Condition }}
policyCondition:
{{- range $key, $value := . }}
"{{ $key }}":
{{- range $innerKey, $innerValue := $value }}
"{{ $innerKey }}": {{ stringOrSlice $innerValue true }}
{{- end }}
{{- end }}
{{- end }}
{{- end }}
secretRef:
name: aws-load-balancer-controller-cluster
namespace: aws-load-balancer-operator
Expand Down
18 changes: 16 additions & 2 deletions cmd/iamctl/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@ import (
"github.com/spf13/cobra"
)

const (
defaultFunction = "GetIAMPolicy"
)

var (
// input file specifies the location for the input json.
inputFile string
Expand All @@ -19,8 +23,14 @@ var (
// pkg specifies the package with which the code is generated.
pkg string

// function specifies the function name with which the code is generated.
function string

// skipMinify specifies whether the minification of the AWS policy has to be skipped.
skipMinify bool

// splitResource splits IAM policy's statement into many with one resource per statement.
splitResource bool
)

// rootCmd represents the base command when called without any subcommands
Expand All @@ -32,7 +42,7 @@ var rootCmd = &cobra.Command{
Also it can produce a CredentialsRequest YAML file which can provision the secret for the controller.
`,
Run: func(cmd *cobra.Command, args []string) {
generateIAMPolicy(inputFile, outputFile, outputCRFile, pkg)
generateIAMPolicy(inputFile, outputFile, outputCRFile, pkg, function)
},
}

Expand All @@ -56,8 +66,12 @@ func init() {

rootCmd.PersistentFlags().StringVarP(&outputCRFile, "output-cr-file", "c", "", "Used to specify output CredentialsRequest YAML file path.")

rootCmd.PersistentFlags().StringVarP(&pkg, "package", "p", "main", "Used to specify output Go file path.")
rootCmd.PersistentFlags().StringVarP(&pkg, "package", "p", "main", "Used to specify the Go package in the output file.")
_ = rootCmd.MarkPersistentFlagRequired("package")

rootCmd.PersistentFlags().StringVarP(&function, "function", "f", defaultFunction, "Used to specify the Go function name in the output file.")

rootCmd.PersistentFlags().BoolVarP(&skipMinify, "no-minify", "n", false, "Used to skip the minification of the output AWS policy.")

rootCmd.PersistentFlags().BoolVarP(&splitResource, "split-resource", "s", false, "Used to split AWS policy's statement into many with one resource per statement.")
}
74 changes: 65 additions & 9 deletions cmd/iamctl/policy.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ func (v *AWSValue) UnmarshalJSON(input []byte) error {

type iamPolicyCondition map[string]iamPolicyConditionKeyValue

type iamPolicyConditionKeyValue map[string]string
type iamPolicyConditionKeyValue map[string]interface{}

// compressionPrefixes defines a list of action prefixes in the policy
// that are going to be compressed using wildcards.
Expand All @@ -54,15 +54,34 @@ var compressionPrefixes = map[string]string{
"elasticloadbalancing:Describe": "elasticloadbalancing:Describe*",
}

func generateIAMPolicy(inputFile, output, outputCR, pkg string) {
generateIAMPolicyFromTemplate(filetemplate, inputFile, output, pkg)
func generateIAMPolicy(inputFile, output, outputCR, pkg, function string) {
generateIAMPolicyFromTemplate(filetemplate, inputFile, output, pkg, function)
if outputCR != "" {
generateIAMPolicyFromTemplate(credentialsRequestTemplate, inputFile, outputCR, pkg)
generateIAMPolicyFromTemplate(credentialsRequestTemplate, inputFile, outputCR, pkg, function)
}
}

func generateIAMPolicyFromTemplate(filetemplate string, inputFile, output, pkg string) {
tmpl, err := template.New("").Parse(filetemplate)
func generateIAMPolicyFromTemplate(filetemplate string, inputFile, output, pkg, function string) {
funcMap := template.FuncMap{
"stringOrSlice": func(value interface{}, yaml bool) string {
if values, slice := value.([]interface{}); slice {
result := ""
for i, v := range values {
if i > 0 {
result += ","
}
result += fmt.Sprintf("%q", v)
}
if yaml {
return "[" + result + "]"
}
return "[]string{" + result + "}"
}
return fmt.Sprintf("%q", value)
},
}

tmpl, err := template.New("").Funcs(funcMap).Parse(filetemplate)
if err != nil {
panic(err)
}
Expand All @@ -79,6 +98,12 @@ func generateIAMPolicyFromTemplate(filetemplate string, inputFile, output, pkg s
panic(fmt.Errorf("failed to parse policy JSON %v", err))
}

if splitResource {
// Splitting policy statement into many with one resource per statement
// because credentials request's resource is not a slice.
policy = split(policy)
}

if !skipMinify {
// Minifying here as a workaround for current limitations
// in credential requests length (2048 max bytes).
Expand All @@ -92,9 +117,16 @@ func generateIAMPolicyFromTemplate(filetemplate string, inputFile, output, pkg s

var in bytes.Buffer
err = tmpl.Execute(&in, struct {
Package string
Statement []policyStatement
}{Package: pkg, Statement: policy.Statement})
Package string
Function string
Definition bool
Statement []policyStatement
}{
Package: pkg,
Function: function,
Definition: function == defaultFunction,
Statement: policy.Statement,
})
if err != nil {
panic(err)
}
Expand Down Expand Up @@ -148,3 +180,27 @@ func minify(policy iamPolicy) iamPolicy {
}
return miniPolicy
}

func split(policy iamPolicy) iamPolicy {
var splitPolicy iamPolicy
splitPolicy.Version = policy.Version

for _, statement := range policy.Statement {
if len(statement.Resource) > 1 {
newStatements := []policyStatement{}
for _, resource := range statement.Resource {
newStatement := policyStatement{
Effect: statement.Effect,
Action: statement.Action,
Resource: AWSValue{resource},
Condition: statement.Condition,
}
newStatements = append(newStatements, newStatement)
}
splitPolicy.Statement = append(splitPolicy.Statement, newStatements...)
} else {
splitPolicy.Statement = append(splitPolicy.Statement, statement)
}
}
return splitPolicy
}
68 changes: 68 additions & 0 deletions hack/controller/controller-credentials-request-minify.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
apiVersion: cloudcredential.openshift.io/v1
kind: CredentialsRequest
metadata:
name: aws-load-balancer-controller
namespace: openshift-cloud-credential-operator
spec:
providerSpec:
apiVersion: cloudcredential.openshift.io/v1
kind: AWSProviderSpec
statementEntries:
- action:
- acm:DescribeCertificate
- acm:ListCertificates
- cognito-idp:DescribeUserPoolClient
- ec2:AuthorizeSecurityGroupIngress
- ec2:CreateSecurityGroup
- ec2:CreateTags
- ec2:DeleteSecurityGroup
- ec2:DeleteTags
- ec2:Describe*
- ec2:GetCoipPoolUsage
- ec2:RevokeSecurityGroupIngress
- elasticloadbalancing:AddListenerCertificates
- elasticloadbalancing:AddTags
- elasticloadbalancing:CreateListener
- elasticloadbalancing:CreateLoadBalancer
- elasticloadbalancing:CreateRule
- elasticloadbalancing:CreateTargetGroup
- elasticloadbalancing:DeleteListener
- elasticloadbalancing:DeleteLoadBalancer
- elasticloadbalancing:DeleteRule
- elasticloadbalancing:DeleteTargetGroup
- elasticloadbalancing:DeregisterTargets
- elasticloadbalancing:Describe*
- elasticloadbalancing:ModifyListener
- elasticloadbalancing:ModifyLoadBalancerAttributes
- elasticloadbalancing:ModifyRule
- elasticloadbalancing:ModifyTargetGroup
- elasticloadbalancing:ModifyTargetGroupAttributes
- elasticloadbalancing:RegisterTargets
- elasticloadbalancing:RemoveListenerCertificates
- elasticloadbalancing:RemoveTags
- elasticloadbalancing:SetIpAddressType
- elasticloadbalancing:SetSecurityGroups
- elasticloadbalancing:SetSubnets
- elasticloadbalancing:SetWebAcl
- iam:CreateServiceLinkedRole
- iam:GetServerCertificate
- iam:ListServerCertificates
- shield:CreateProtection
- shield:DeleteProtection
- shield:DescribeProtection
- shield:GetSubscriptionState
- waf-regional:AssociateWebACL
- waf-regional:DisassociateWebACL
- waf-regional:GetWebACL
- waf-regional:GetWebACLForResource
- wafv2:AssociateWebACL
- wafv2:DisassociateWebACL
- wafv2:GetWebACL
- wafv2:GetWebACLForResource
effect: Allow
resource: "*"
secretRef:
name: aws-load-balancer-controller-cluster
namespace: aws-load-balancer-operator
serviceAccountNames:
- aws-load-balancer-controller-cluster
Loading