Allow network whitelisting on the service level #3581
Conversation
beastawakens
force-pushed
the
service-whitelist-support
branch
from
22c8aeb to
4aed578
Compare
Codecov ReportBase: 36.19% // Head: 36.22% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## master #3581 +/- ##
==========================================
+ Coverage 36.19% 36.22% +0.02%
==========================================
Files 168 168
Lines 18413 18418 +5
==========================================
+ Hits 6665 6672 +7
+ Misses 10616 10613 -3
- Partials 1132 1133 +1
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
Twsouza
force-pushed
the
service-whitelist-support
branch
from
4aed578 to
ae88e4f
Compare
| @@ -238,7 +238,7 @@ | |||
| {{ end }} | |||
| "Properties": { | |||
| "Actions": [ { "Type": "forward", "TargetGroupArn": { "Ref": "BalancerTargetGroup{{ if .Internal }}Internal{{ end }}" } } ], | |||
| "Conditions": [ { "Field": "host-header", "Values": [ { "Fn::Join": [ ".", [ "{{$.App}}-{{.Name}}", { "Fn::ImportValue": { "Fn::Sub": "${Rack}:{{ router .Name $.Manifest }}Host" } } ] ] } ] } ], | |||
| "Conditions": [ { "Field": "host-header", "Values": [ { "Fn::Join": [ ".", [ "{{$.App}}-{{.Name}}", { "Fn::ImportValue": { "Fn::Sub": "${Rack}:{{ router .Name $.Manifest }}Host" } } ] ] } ] }, { "Field": "source-ip", "SourceIpConfig": { "Values": [ {{ range safeWhitelist .Whitelist }} "{{ . }}", {{ end }} ] } } ], | |||
There was a problem hiding this comment.
@beastawakens the e2e tests got an error here:
Promoting RHQCJJDYHME... ERROR: json syntax error: line 248 pos 241: invalid character ']' looking for beginning of value: "Conditions": [ { "Field": "host-header", "Values": [ { "Fn::Join": [ ".", [ "ci2-web", { "Fn::ImportValue": { "Fn::Sub": "${Rack}:RouterHost" } } ] ] } ] }, { "Field": "source-ip", "SourceIpConfig": { "Values": [ "0.0.0.0/0", ] } } ],
There was a problem hiding this comment.
Uuurgh, I know the syntax is a nested mess but that's CloudFormation for you. I'm not seeing an extra closing bracket though 🤔 there's 5 opening, and 5 closing by my reckoning? Can you spot it?
There was a problem hiding this comment.
I don't see any extra closing. I think I've found the problem when you create the app with an empty whitelist it will create "SourceIpConfig": { "Values": [ "0.0.0.0/0", ] } } ], you can see that will create an invalid comma in the values list.
|
@beastawakens if you could also create some tests to assert this new functionality we would greatly appretiate |
I think this syntax is correct but I'd recommend some manual testing of this, just to be sure!