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

feat: add existing secret functionality to Falcon Integration Gateway #294

Closed
wants to merge 4 commits into from
Closed

feat: add existing secret functionality to Falcon Integration Gateway #294

wants to merge 4 commits into from

Conversation

mkeats-clumio
Copy link

@mkeats-clumio mkeats-clumio commented May 21, 2024

Fixes #283

This adds a existingSecret value in the same style as the other charts in this repo.

It also adds the option to pass in arbitrary environment variables in order to manage the configurations that are embedded in the secret.

Comment on lines 1 to 30
apiVersion: v1
kind: ConfigMap
metadata:
name: {{ include "falcon-integration-gateway.fullname" . }}-env
namespace: {{ .Release.Namespace }}
labels:
{{- include "falcon-integration-gateway.labels" . | nindent 4 }}
data:
FALCON_CLOUD_REGION: {{ .Values.falcon.cloud_region | quote }}
{{- if .Values.push.aws_security_hub.enabled }}
AWS_REGION: {{ .Values.push.aws_security_hub.region | quote }}
{{- end }}
{{- if .Values.push.azure_log_analytics.enabled }}
WORKSPACE_ID: {{ .Values.push.azure_log_analytics.workspace_id | quote }}
PRIMARY_KEY: {{ .Values.push.azure_log_analytics.primary_key | quote }}
ARC_AUTODISCOVERY: {{ .Values.push.azure_log_analytics.arc_autodiscovery | toString | quote }}
{{- end }}
{{- if .Values.push.chronicle.enabled }}
CHRONICLE_REGION: {{ .Values.push.chronicle.region | quote }}
GOOGLE_SECURITY_KEY: {{ .Values.push.chronicle.security_key | quote }}
{{- end }}
{{- if .Values.push.cloudtrail_lake.enabled }}
CLOUDTRAIL_LAKE_CHANNEL_ARN: {{ .Values.push.cloudtrail_lake.channel_arn | quote }}
CLOUDTRAIL_LAKE_REGION: {{ .Values.push.cloudtrail_lake.region | quote }}
{{- end }}
{{- if .Values.push.vmware_workspace_one.enabled }}
SYSLOG_HOST: {{ .Values.push.vmware_workspace_one.syslog_host | quote }}
SYSLOG_PORT: {{ .Values.push.vmware_workspace_one.syslog_port }}
WORKSPACEONE_TOKEN: {{ .Values.push.vmware_workspace_one.token | quote }}
{{- end }}
Copy link
Contributor

Choose a reason for hiding this comment

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

This should stay where it originally was in a secret since it still contains sensitive data. So please drop this configmap addition as well as the configmap file rename.

Copy link
Author

@mkeats-clumio mkeats-clumio Jun 13, 2024

Choose a reason for hiding this comment

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

The intent is to move the non-sensitive data out of the secret and into a config map where it belongs. If I set up an existing secret I don't want to be managing SYSLOG_HOST there for example. It should only contain sensitive data so I can keep the config managed in helm.

To be clear my specific situation is for managing the values with a gitops workflow. I need to manage all configuration except secrets within the repo. Simply replacing the secret with an existingSecret does not work for this pattern since I won't be able to manage AWS_REGION in the repo. Looking at the secret almost none of it is actually sensitive so I attempted to be more consistent with every other chart in this repository. However there is already a configmap that is directly mounted to a config file.

Another option is to use an env: key in the values to add additional needed vars that aren't configurable like confirm_instance instead of explicitly listing them out in the template. This might be the most minimal change that would allow the existing logic to remain.

It looks like the only sensitive fields here are GOOGLE_SECURITY_KEY and WORKSPACEONE_TOKEN however. Can you clarify if anything else is sensitive? Maybe the cloudtrail lake ARN

Copy link
Contributor

Choose a reason for hiding this comment

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

You'd want to update the config.ini and not create a new configmap. Technically, secrets are not secrets in k8s, and the use of secretRef should be avoided due to secrets as env var exposure, credential dumping, and exporting plaintext via logs. There are other sensitive values like WORKSPACE_ID and PRIMARY_KEY

Copy link
Author

Choose a reason for hiding this comment

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

See changes

Copy link
Author

@mkeats-clumio mkeats-clumio Jun 13, 2024

Choose a reason for hiding this comment

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

The existing chart uses secretRef I just made the name here configurable.

In order to reduce scope of changes I moved to use env instead of attempting to figure out what was correctly a secret or not. This allows me to set AWS_REGION in the vars as well as the other missing env vars. I wasn't sure whether to place it in the global level or under falcon

@redhatrises
Copy link
Contributor

@mkeats-clumio see if #297 fits your needs. Turns out there was a good amount of updating that needed to happen for the config. I have also added your existing secret code there.

@mkeats-clumio
Copy link
Author

@redhatrises that looks perfect. I'll test it out shortly. I'll close this PR after I confirm

@mkeats-clumio
Copy link
Author

#297 Looks good, closing this one

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.

Integration Gateway does not take existing secret
3 participants