Skip to content

Commit fedc7a4

Browse files
authored
late initialize Role.Description (#79)
The IAM CreateRole API response always has a nil Description field, even if the CreateRole API request contains a non-nil Description field. This is totally broken behaviour that also exists for the CreatePolicy API but apparently won't be fixed by IAM. So, this commit papers over that flaw in the API by late-initializing Role.Description to the (correct) non-nil value from the DescribeRole API response. Fixes Issue aws-controllers-k8s/community#1772 By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
1 parent 84457a1 commit fedc7a4

File tree

10 files changed

+39
-18
lines changed

10 files changed

+39
-18
lines changed
Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
11
ack_generate_info:
2-
build_date: "2023-05-01T22:15:40Z"
3-
build_hash: 6657565bb742e5cd4cd340d01d5e4786b5fbabc0
4-
go_version: go1.19
5-
version: v0.26.0
2+
build_date: "2023-05-03T16:53:00Z"
3+
build_hash: 06bf1325851814e1f8004dc70fb50f5023fdf24c
4+
go_version: go1.19.4
5+
version: v0.26.0-1-g06bf132
66
api_directory_checksum: 26341f700d12dfcd4033cf4203492fa381daa7b0
77
api_version: v1alpha1
88
aws_sdk_go_version: v1.44.93
99
generator_config_info:
10-
file_checksum: 0a343bca2be29176e9388a2cc3d54461162c5984
10+
file_checksum: 0dfb99b66da90fef5d3bd6eb512243bf87f6d16d
1111
original_file_name: generator.yaml
1212
last_modification:
1313
reason: API generation

apis/v1alpha1/generator.yaml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,9 @@ resources:
167167
set:
168168
# The input and output shapes are different...
169169
- from: PermissionsBoundary.PermissionsBoundaryArn
170+
Description:
171+
# See above in Policy resource about why this is here.
172+
late_initialize: {}
170173
Path:
171174
late_initialize: {}
172175
# In order to support attaching zero or more policies to a role, we use

generator.yaml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,9 @@ resources:
167167
set:
168168
# The input and output shapes are different...
169169
- from: PermissionsBoundary.PermissionsBoundaryArn
170+
Description:
171+
# See above in Policy resource about why this is here.
172+
late_initialize: {}
170173
Path:
171174
late_initialize: {}
172175
# In order to support attaching zero or more policies to a role, we use

helm/crds/services.k8s.aws_adoptedresources.yaml

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -145,10 +145,7 @@ spec:
145145
blockOwnerDeletion:
146146
description: If true, AND if the owner has the "foregroundDeletion"
147147
finalizer, then the owner cannot be deleted from the
148-
key-value store until this reference is removed. See
149-
https://kubernetes.io/docs/concepts/architecture/garbage-collection/#foreground-deletion
150-
for how the garbage collector interacts with this
151-
field and enforces the foreground deletion. Defaults
148+
key-value store until this reference is removed. Defaults
152149
to false. To set this field, a user needs "delete"
153150
permission of the owner, otherwise 422 (Unprocessable
154151
Entity) will be returned.

helm/templates/_helpers.tpl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ If release name contains chart name it will be used as a full name.
3333

3434
{{- define "watch-namespace" -}}
3535
{{- if eq .Values.installScope "namespace" -}}
36-
{{- .Release.Namespace -}}
36+
{{ .Values.watchNamespace | default .Release.Namespace }}
3737
{{- end -}}
3838
{{- end -}}
3939

helm/values.schema.json

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -196,6 +196,9 @@
196196
"type": "string",
197197
"enum": ["cluster", "namespace"]
198198
},
199+
"watchNamespace": {
200+
"type": "string"
201+
},
199202
"resourceTags": {
200203
"type": "array",
201204
"items": {

helm/values.yaml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,10 @@ log:
7272
# cluster wide.
7373
installScope: cluster
7474

75+
# Set the value of the "namespace" to be watched by the controller
76+
# This value is only used when the `installScope` is set to "namespace". If left empty, the default value is the release namespace for the chart.
77+
watchNamespace: ""
78+
7579
resourceTags:
7680
# Configures the ACK service controller to always set key/value pairs tags on
7781
# resources that it manages.

pkg/resource/role/manager.go

Lines changed: 7 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
if ko.Spec.AssumeRolePolicyDocument != nil {
2-
if doc, err := decodeDocument(*ko.Spec.AssumeRolePolicyDocument); err != nil {
3-
return nil, err
4-
} else {
5-
ko.Spec.AssumeRolePolicyDocument = &doc
6-
}
7-
}
2+
if doc, err := decodeDocument(*ko.Spec.AssumeRolePolicyDocument); err != nil {
3+
return nil, err
4+
} else {
5+
ko.Spec.AssumeRolePolicyDocument = &doc
6+
}
7+
}
88
ackcondition.SetSynced(&resource{ko}, corev1.ConditionFalse, nil, nil)

test/e2e/tests/test_role.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
CHECK_STATUS_WAIT_SECONDS = 10
3333
MODIFY_WAIT_AFTER_SECONDS = 10
3434
MAX_SESS_DURATION = 3600 # Note: minimum of 3600 seconds...
35+
ROLE_DESC = "a simple role"
3536

3637

3738
@pytest.fixture(scope="module")
@@ -41,7 +42,7 @@ def simple_role():
4142

4243
replacements = REPLACEMENT_VALUES.copy()
4344
replacements['ROLE_NAME'] = role_name
44-
replacements['ROLE_DESCRIPTION'] = role_desc
45+
replacements['ROLE_DESCRIPTION'] = ROLE_DESC
4546
replacements['MAX_SESSION_DURATION'] = str(MAX_SESS_DURATION)
4647

4748
resource_data = load_resource(
@@ -91,6 +92,10 @@ def test_crud(self, simple_role):
9192
assert 'spec' in cr
9293
assert 'maxSessionDuration' in cr['spec']
9394
assert cr['spec']['maxSessionDuration'] == MAX_SESS_DURATION
95+
# Check that the Description field has not been removed.
96+
# See: https://github.com/aws-controllers-k8s/community/issues/1772
97+
assert 'description' in cr['spec']
98+
assert cr['spec']['description'] == ROLE_DESC
9499

95100
condition.assert_synced(ref)
96101

0 commit comments

Comments
 (0)