-
Notifications
You must be signed in to change notification settings - Fork 82
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
Add example for AWS feature: Security Groups for Pods #1429
Conversation
Does the PR have any schema changes?Looking good! No breaking changes found. |
1b663de
to
19656ca
Compare
}, | ||
}, { provider: kube, dependsOn: [nginx, callerSgp] }); | ||
|
||
function configureClusterAccess(name: string, cluster: eks.Cluster, sg: aws.ec2.SecurityGroup) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this live in a helper file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's an important part of this example (allowing the pods to reach things like coredns). Putting it into another file would hide that aspect IMO
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm ok. I'm surprised it works while written at the bottom of the file but I guess that's how Node works.
}, | ||
}, { provider: kube }); | ||
|
||
// Create a job that is not allowed to curl the nginx service. The job will fail if it can reach the service. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This feels like a counter-example to test something instead of a best-practice example of how to GTD, or am I misunderstanding? Does it belong under ./examples?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd say showing how traffic can be restricted is an important part of a firewall example.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, yeah I concede the point. A few comments to that end then perhaps? Maybe in the readme? I still get confused by mixing tests and examples in one.
const callerSg = new aws.ec2.SecurityGroup("caller", { | ||
vpcId: eksVpc.vpcId, | ||
}); | ||
// Allow all traffic between the cluster and the caller SecurityGroup |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider consistent spacing. My eye not trained on TypeScript style but should this have an empty line before //?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Uh, that was by accident. Somehow format on save was turned off
@@ -0,0 +1,3 @@ | |||
# EKS Pod Security Groups | |||
|
|||
Demonstrates how to configure your Pulumi EKS Cluster to use Pod Security Groups. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It could be cute to include a mermaid diagram that shows what talks to what in this example, but it may be a lot of work. Wonder if AI can do a quick pass at it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea! I added one
This adds an example (and acceptance test) for the AWS feature: Security Groups for Pods. The configuration is derived from this AWS example: https://docs.aws.amazon.com/eks/latest/userguide/security-groups-pods-deployment.html
This PR has been shipped in release v3.0.0-beta.2. |
This adds an example (and acceptance test) for the AWS feature: Security Groups for Pods. The configuration is derived from this AWS example: https://docs.aws.amazon.com/eks/latest/userguide/security-groups-pods-deployment.html
This adds an example (and acceptance test) for the AWS feature: Security Groups for Pods.
The configuration is derived from this AWS example: https://docs.aws.amazon.com/eks/latest/userguide/security-groups-pods-deployment.html