Skip to content

Commit 6513670

Browse files
kddejongkddejong
authored
A set of cleanup fixes (#4014)
* A set of cleanup fixes * Limit where github actions run
1 parent 633ff85 commit 6513670

File tree

13 files changed

+147
-24
lines changed

13 files changed

+147
-24
lines changed

‎.github/workflows/cd-pypi.yaml

+1
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ on:
77
jobs:
88
deploy:
99
runs-on: ubuntu-latest
10+
if: github.repository == 'aws-cloudformation/cfn-lint'
1011
steps:
1112
- uses: actions/checkout@v4
1213
- name: Set up Python

‎.github/workflows/cd-release.yaml

+1
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ on:
77
jobs:
88
build:
99
runs-on: ubuntu-latest
10+
if: github.repository == 'aws-cloudformation/cfn-lint'
1011
steps:
1112
- name: Checkout
1213
uses: actions/checkout@v4

‎.github/workflows/maintenance-v0.yaml

+1
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ on:
66
jobs:
77
job:
88
runs-on: ubuntu-latest
9+
if: github.repository == 'aws-cloudformation/cfn-lint'
910
steps:
1011
- uses: actions/checkout@v4
1112
with:

‎.github/workflows/maintenance-v1.yaml

+1
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ on:
66
jobs:
77
job:
88
runs-on: ubuntu-latest
9+
if: github.repository == 'aws-cloudformation/cfn-lint'
910
steps:
1011
- uses: actions/checkout@v4
1112
with:

‎scripts/update_schemas_manually.py

+2-2
Original file line numberDiff line numberDiff line change
@@ -593,11 +593,11 @@
593593
path="/definitions/StageDeclaration/properties/Actions",
594594
),
595595
Patch(
596-
values={"pattern": "^[0-9A-Za-z_-]{1,9}$"},
596+
values={"pattern": "^[0-9A-Za-z_\-]{1,9}$"},
597597
path="/definitions/ActionTypeId/properties/Version",
598598
),
599599
Patch(
600-
values={"pattern": "^[A-Za-z0-9.@-_]{1,100}$"},
600+
values={"pattern": "^[A-Za-z0-9.@\-_]{1,100}$"},
601601
path="/definitions/ActionDeclaration/properties/Name",
602602
),
603603
],

‎src/cfnlint/data/schemas/patches/extensions/all/aws_codepipeline_pipeline/manual.json

+2-2
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,12 @@
1010
{
1111
"op": "add",
1212
"path": "/definitions/ActionDeclaration/properties/Name/pattern",
13-
"value": "^[A-Za-z0-9.@-_]{1,100}$"
13+
"value": "^[A-Za-z0-9.@\\-_]{1,100}$"
1414
},
1515
{
1616
"op": "add",
1717
"path": "/definitions/ActionTypeId/properties/Version/pattern",
18-
"value": "^[0-9A-Za-z_-]{1,9}$"
18+
"value": "^[0-9A-Za-z_\\-]{1,9}$"
1919
},
2020
{
2121
"op": "add",

‎src/cfnlint/data/schemas/providers/ap_southeast_5/aws-codepipeline-pipeline.json

+2-2
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@
2525
"uniqueItems": true
2626
},
2727
"Name": {
28-
"pattern": "^[A-Za-z0-9.@-_]{1,100}$",
28+
"pattern": "^[A-Za-z0-9.@\\-_]{1,100}$",
2929
"type": "string"
3030
},
3131
"Namespace": {
@@ -76,7 +76,7 @@
7676
"type": "string"
7777
},
7878
"Version": {
79-
"pattern": "^[0-9A-Za-z_-]{1,9}$",
79+
"pattern": "^[0-9A-Za-z_\\-]{1,9}$",
8080
"type": "string"
8181
}
8282
},

‎src/cfnlint/data/schemas/providers/ap_southeast_7/aws-codepipeline-pipeline.json

+2-2
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@
2525
"uniqueItems": true
2626
},
2727
"Name": {
28-
"pattern": "^[A-Za-z0-9.@-_]{1,100}$",
28+
"pattern": "^[A-Za-z0-9.@\\-_]{1,100}$",
2929
"type": "string"
3030
},
3131
"Namespace": {
@@ -76,7 +76,7 @@
7676
"type": "string"
7777
},
7878
"Version": {
79-
"pattern": "^[0-9A-Za-z_-]{1,9}$",
79+
"pattern": "^[0-9A-Za-z_\\-]{1,9}$",
8080
"type": "string"
8181
}
8282
},

‎src/cfnlint/data/schemas/providers/ca_west_1/aws-codepipeline-pipeline.json

+2-2
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@
2525
"uniqueItems": true
2626
},
2727
"Name": {
28-
"pattern": "^[A-Za-z0-9.@-_]{1,100}$",
28+
"pattern": "^[A-Za-z0-9.@\\-_]{1,100}$",
2929
"type": "string"
3030
},
3131
"Namespace": {
@@ -76,7 +76,7 @@
7676
"type": "string"
7777
},
7878
"Version": {
79-
"pattern": "^[0-9A-Za-z_-]{1,9}$",
79+
"pattern": "^[0-9A-Za-z_\\-]{1,9}$",
8080
"type": "string"
8181
}
8282
},

‎src/cfnlint/data/schemas/providers/us_east_1/aws-codepipeline-pipeline.json

+2-2
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@
3232
"uniqueItems": true
3333
},
3434
"Name": {
35-
"pattern": "^[A-Za-z0-9.@-_]{1,100}$",
35+
"pattern": "^[A-Za-z0-9.@\\-_]{1,100}$",
3636
"type": "string"
3737
},
3838
"Namespace": {
@@ -100,7 +100,7 @@
100100
"type": "string"
101101
},
102102
"Version": {
103-
"pattern": "^[0-9A-Za-z_-]{1,9}$",
103+
"pattern": "^[0-9A-Za-z_\\-]{1,9}$",
104104
"type": "string"
105105
}
106106
},

‎src/cfnlint/rules/resources/iam/ResourcePolicy.py

+16
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,9 @@
33
SPDX-License-Identifier: MIT-0
44
"""
55

6+
from typing import Any
7+
8+
from cfnlint.jsonschema import ValidationResult, Validator
69
from cfnlint.rules.resources.iam.Policy import Policy
710

811

@@ -30,3 +33,16 @@ def __init__(self):
3033
"resource",
3134
"policy_resource.json",
3235
)
36+
37+
def validate(
38+
self,
39+
validator: Validator,
40+
policy_type: Any,
41+
policy: Any,
42+
schema: dict[str, Any],
43+
) -> ValidationResult:
44+
for err in super().validate(validator, policy_type, policy, schema):
45+
if validator.context.path.cfn_path[1] == "AWS::S3::BucketPolicy":
46+
if err.validator == "uniqueKeys" and err.schema_path[-2] == "Statement":
47+
continue
48+
yield err

‎test/integration/test_schema_files.py

+1
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ class TestSchemaFiles(TestCase):
4040
"Outputs/*/Value",
4141
"Parameters",
4242
"Parameters/*",
43+
"Parameters/*/AllowedPattern",
4344
"Resources",
4445
"Resources/*",
4546
"Resources/*/Condition",

‎test/unit/rules/resources/iam/test_resource_policy.py

+114-12
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,12 @@
33
SPDX-License-Identifier: MIT-0
44
"""
55

6+
from collections import deque
67
from unittest import TestCase
78

8-
from cfnlint.context import Context
9+
from cfnlint.context import Context, Path
910
from cfnlint.helpers import FUNCTIONS
10-
from cfnlint.jsonschema import CfnTemplateValidator
11+
from cfnlint.jsonschema import CfnTemplateValidator, ValidationError
1112
from cfnlint.rules.resources.iam.ResourcePolicy import ResourcePolicy
1213

1314

@@ -20,7 +21,13 @@ def setUp(self):
2021

2122
def test_object_basic(self):
2223
"""Test Positive"""
23-
validator = CfnTemplateValidator()
24+
validator = CfnTemplateValidator({}).evolve(
25+
context=Context(
26+
path=Path(
27+
cfn_path=deque(["Resources", "AWS::S3::BucketPolicy", "Properties"])
28+
)
29+
)
30+
)
2431

2532
policy = {"Version": "2012-10-18"}
2633

@@ -30,15 +37,21 @@ def test_object_basic(self):
3037
)
3138
)
3239
self.assertEqual(len(errs), 2, errs)
33-
self.assertEqual(errs[0].message, "'Statement' is a required property")
34-
self.assertListEqual(list(errs[0].path), [])
3540
self.assertEqual(
36-
errs[1].message, "'2012-10-18' is not one of ['2008-10-17', '2012-10-17']"
41+
errs[0].message, "'2012-10-18' is not one of ['2008-10-17', '2012-10-17']"
3742
)
38-
self.assertListEqual(list(errs[1].path), ["Version"])
43+
self.assertListEqual(list(errs[0].path), ["Version"])
44+
self.assertEqual(errs[1].message, "'Statement' is a required property")
45+
self.assertListEqual(list(errs[1].path), [])
3946

4047
def test_object_multiple_effect(self):
41-
validator = CfnTemplateValidator()
48+
validator = CfnTemplateValidator({}).evolve(
49+
context=Context(
50+
path=Path(
51+
cfn_path=deque(["Resources", "AWS::S3::BucketPolicy", "Properties"])
52+
)
53+
)
54+
)
4255

4356
policy = {
4457
"Version": "2012-10-17",
@@ -91,7 +104,12 @@ def test_object_multiple_effect(self):
91104

92105
def test_object_statements(self):
93106
validator = CfnTemplateValidator({}).evolve(
94-
context=Context(functions=FUNCTIONS)
107+
context=Context(
108+
functions=FUNCTIONS,
109+
path=Path(
110+
cfn_path=deque(["Resources", "AWS::S3::BucketPolicy", "Properties"])
111+
),
112+
)
95113
)
96114

97115
policy = {
@@ -138,7 +156,13 @@ def test_object_statements(self):
138156

139157
def test_string_statements(self):
140158
"""Test Positive"""
141-
validator = CfnTemplateValidator()
159+
validator = CfnTemplateValidator({}).evolve(
160+
context=Context(
161+
path=Path(
162+
cfn_path=deque(["Resources", "AWS::S3::BucketPolicy", "Properties"])
163+
)
164+
)
165+
)
142166

143167
# ruff: noqa: E501
144168
policy = """
@@ -183,7 +207,12 @@ def test_string_statements(self):
183207

184208
def test_principal_wildcard(self):
185209
validator = CfnTemplateValidator({}).evolve(
186-
context=Context(functions=FUNCTIONS)
210+
context=Context(
211+
functions=FUNCTIONS,
212+
path=Path(
213+
cfn_path=deque(["Resources", "AWS::S3::BucketPolicy", "Properties"])
214+
),
215+
)
187216
)
188217

189218
policy = {
@@ -227,13 +256,59 @@ def test_principal_wildcard(self):
227256

228257
def test_assumed_role(self):
229258
validator = CfnTemplateValidator({}).evolve(
230-
context=Context(functions=FUNCTIONS)
259+
context=Context(
260+
functions=FUNCTIONS,
261+
path=Path(
262+
cfn_path=deque(["Resources", "AWS::S3::BucketPolicy", "Properties"])
263+
),
264+
)
265+
)
266+
267+
policy = {
268+
"Version": "2012-10-17",
269+
"Statement": [
270+
{
271+
"Effect": "Allow",
272+
"Action": "*",
273+
"Resource": "arn:aws:s3:::bucket",
274+
"Principal": {
275+
"AWS": "arn:aws:sts::123456789012:assumed-role/rolename/rolesessionname"
276+
},
277+
},
278+
],
279+
}
280+
281+
errs = list(
282+
self.rule.validate(
283+
validator=validator, policy=policy, schema={}, policy_type=None
284+
)
285+
)
286+
self.assertListEqual(errs, [])
287+
288+
def test_duplicate_sid(self):
289+
validator = CfnTemplateValidator({}).evolve(
290+
context=Context(
291+
functions=FUNCTIONS,
292+
path=Path(
293+
cfn_path=deque(["Resources", "AWS::S3::BucketPolicy", "Properties"])
294+
),
295+
)
231296
)
232297

233298
policy = {
234299
"Version": "2012-10-17",
235300
"Statement": [
236301
{
302+
"Sid": "A",
303+
"Effect": "Allow",
304+
"Action": "*",
305+
"Resource": "arn:aws:s3:::bucket",
306+
"Principal": {
307+
"AWS": "arn:aws:sts::123456789012:assumed-role/rolename/rolesessionname"
308+
},
309+
},
310+
{
311+
"Sid": "A",
237312
"Effect": "Allow",
238313
"Action": "*",
239314
"Resource": "arn:aws:s3:::bucket",
@@ -250,3 +325,30 @@ def test_assumed_role(self):
250325
)
251326
)
252327
self.assertListEqual(errs, [])
328+
329+
# Fail on SNS topic
330+
validator = CfnTemplateValidator({}).evolve(
331+
context=Context(
332+
functions=FUNCTIONS,
333+
path=Path(
334+
cfn_path=deque(["Resources", "AWS::SNS::TopicPolicy", "Properties"])
335+
),
336+
)
337+
)
338+
errs = list(
339+
self.rule.validate(
340+
validator=validator, policy=policy, schema={}, policy_type=None
341+
)
342+
)
343+
self.assertListEqual(
344+
errs,
345+
[
346+
ValidationError(
347+
"array items are not unique for keys ['Sid']",
348+
validator="uniqueKeys",
349+
schema_path=deque(["properties", "Statement", "uniqueKeys"]),
350+
path=deque(["Statement"]),
351+
rule=ResourcePolicy(),
352+
)
353+
],
354+
)

0 commit comments

Comments
 (0)