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

Snowplow Collector #16

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Snowplow Collector #16

wants to merge 3 commits into from

Conversation

jbeemster
Copy link
Member

This chart supports deploying the collector locally and to both AWS & GCP via TargetGroup bindings & NEG bindings. It also support custom IAM role passthrough for access to cloud systems.

Further to this to address AkkaHTTP TLS Actor bug it allows for the seamless deployment of an NGINX sidecar deployment with optional TLS support by passing in the certificate parts (and therefore binding a secondary port to the service).

Like with the Iglu Server no Ingress rules are defined as we are expecting to use external load balancers in production - nothing to stop someone from adding that however!


volumes:
- configMap:
defaultMode: 420
Copy link

Choose a reason for hiding this comment

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

why this defaultMode? I understand giving read permissions to user, but write permissions to group? 🤔

Comment on lines +121 to +125
resources:
limits:
memory: 2018Mi
requests:
cpu: 400m
Copy link

Choose a reason for hiding this comment

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

nitpick: It's great to see some guidance on resource values, but best practice is that this should not be hardcoded. Some people might have very low-power clusters or collectors that are handling a very large amount of data and need more resources.

"sqs:SendMessage",
"sqs:ListQueues"
],
"Resource": "*",
Copy link

Choose a reason for hiding this comment

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

I would suggest mentioning that best security policy is to restrict the resource down to just what you need. For example, if your kinesis streams were called good and bad and your AWS account id was 333221111111, then you would do:

            "arn:aws:kinesis:us-east-1:333221111111:stream/bad",
            "arn:aws:kinesis:us-east-1:333221111111:stream/good"

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah thats a fair point - I have done this in the OS Terraform Modules - will replicate it here.

Comment on lines +159 to +165
"kinesis:DescribeStream",
"kinesis:DescribeStreamSummary",
"kinesis:List*",
"kinesis:Put*",
"sqs:GetQueueUrl",
"sqs:SendMessage",
"sqs:ListQueues"
Copy link

Choose a reason for hiding this comment

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

I like the restricted permissions here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants