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

Deprecate Falco's chart integrations in favor of falcosidekick #123

Closed
3 of 4 tasks
leogr opened this issue Oct 14, 2020 · 15 comments · Fixed by #151
Closed
3 of 4 tasks

Deprecate Falco's chart integrations in favor of falcosidekick #123

leogr opened this issue Oct 14, 2020 · 15 comments · Fixed by #151
Assignees
Labels
kind/feature New feature or request

Comments

@leogr
Copy link
Member

leogr commented Oct 14, 2020

Motivation

Falco's chart comes with various thirdy-party integrations that seems to be not maintained anymore.

Moreover, the current implementation of those integrations relies on several docker images that live outside the falcosecurity org:

image: sysdig/falco-nats:latest
,
image: sysdig/falco-sns:latest
,
image: sysdiglabs/falco-pubsub:latest

That makes Falco's chart hard to maintain Falco's chart too (here is an example).

Finally, falcosidekick (that's actively maintained, lives inside the falcosecurity org, and its chart is already inside this repository) already provides some of those integrations (missing ones are coming soon).

Feature

Remove the following integrations from the Falco's chart:

  • gcscc (i.e., Google Cloud Security Command Center)
  • natsOutput
  • snsOutput
  • pubsubOutput (i.e., Google Cloud Pub/Sub)

Finally, document how Falco's chart can integrate with those services by using falcosidekick's.

Alternatives

Do nothing. But sooner or later the current integrations will not work anymore.

Additional context

Service supported by falcosidekick:

  • NATS
  • SNS (recently added)
  • PubSub (work already in progress)
  • GCloud Security Command Center

Slack channel discussion 👇
https://kubernetes.slack.com/archives/CMWH3EH32/p1602690485207200

cc @Issif @nibalizer

@leogr leogr added the kind/feature New feature or request label Oct 14, 2020
@leogr leogr changed the title Deprecate Falco's chart integration in favor of falcosidekick Deprecate Falco's chart integrations in favor of falcosidekick Oct 14, 2020
@leogr
Copy link
Member Author

leogr commented Oct 14, 2020

cc'ing folks that might be using those integrations:
@fjudith @dza89 @NingPekin

@nibalizer
Copy link
Contributor

I greatly agree, we can better support these kinds of things in the sidekick. I'd even say we should default the http output option to http://falco-sidekick:2801 or similar.

@Issif
Copy link
Member

Issif commented Oct 14, 2020

I even thought about integrating falcosidekick in falco's chart, as a dependency, what do you think about?

@nibalizer
Copy link
Contributor

I'm not sure. I would like a single entry point for users. However I feel that bloated code and tons of options doesn't serve the users well. Separation is good, however Helm doesn't seem to have the best tooling to support collections of charts working together.

I found these two things:

Neither of which really looks all that great. It does seem like we could use

condition: falcosidekick.enabled

inside the main Falco chart to turn on http output to port 2801. That might be confusing for the users if http output is disabled in their values.yaml but enabled after they apply the chart.

I'm not against folding the sidekick chart into the main falco chart, I just worry that we're replacing one extension with another, and that in the future we'll probably be shy to change defaults in the falco chart and more willing to iterate and be disruptive to users in a separate chart.

@Issif
Copy link
Member

Issif commented Oct 14, 2020

I agree. I just wondered if we could just enable the import of falcosidekick"s chart with a boolean, but it doesn't seem possible with helm. When all will be ready in falcosidekick, its chart and this. I will write the relative documentation.

@Issif
Copy link
Member

Issif commented Oct 14, 2020

For GCloud Security Command Center, I checked the current implementation, it's just a simple curl. The webhook output in falcosidekick will work, it only needs a way to specify the Authorization Token as header, and this is possible since tonight, in that branch : https://github.com/falcosecurity/falcosidekick/tree/authorization-header.

After implementation of PubSub, falcosidekick will be fully able to replace these components.

@leogr
Copy link
Member Author

leogr commented Oct 29, 2020

Hey

I would like to introduce a deprecation notice in the Falco chart right now. WDYT?

@leogr leogr self-assigned this Oct 29, 2020
@Issif
Copy link
Member

Issif commented Oct 29, 2020

@leogr It could be add in _helpers.tpl, I agree.

@leogr
Copy link
Member Author

leogr commented Nov 24, 2020

Update:
I believe the only missing piece now is the GCloud Security Command Center. So I'm starting to clean up the chart at this point.

@AbirHamzi
Copy link

AbirHamzi commented Dec 12, 2020

Hello everyone, is it possible to introduce a readiness probe in falco to check that falcosidekick is available (when enabled ofc ) something like this maybe :

    readinessProbe:
      tcpSocket:
        host: falcosidekick
        port: 2801

I think this should be handled from falco side whenever a http output enabled a readiness probe should be added I mean not only for falcosidekick isn't ?

@Issif
Copy link
Member

Issif commented Dec 12, 2020

Hi @AbirHamzi,

For me, it's not the duty of falco to check if its backends are up and running. For the moment, falcosidekick is the "official" output extender but it may change in future if someone brings a better tool. We should not intricate them.

In kubernetes, falcosidekick deployment already has readiness and liveness probes:

          livenessProbe:
            httpGet:
              path: /ping
              port: http
            initialDelaySeconds: 10
            periodSeconds: 5
          readinessProbe:
            httpGet:
              path: /ping
              port: http
            initialDelaySeconds: 10
            periodSeconds: 5

And when you set in falco's config:

httpOutput:
  enabled: true
  url: "http://falcosidekick:2801/"

You use a kubernetes service that uses these probes.

@AbirHamzi
Copy link

AbirHamzi commented Dec 12, 2020

@Issif okay if you see so, but just to clarify my point of view, I meant that from falco side if we enable the http output (for falcosidekick or any other server) we may set unreachable url by mistake (I did that :() we can see that in falco logs but I thought it's better to introduce it as readiness probe to save debugging time and we read values from values.yaml like following:

 {{- if .Values.httpOutput.enabled }}
   readinessProbe:
      tcpSocket:
        host: {{ .Values.httpOutput.host }}
        port: {{ .Values.httpOutput.port }}
{{- end }}

@leogr
Copy link
Member Author

leogr commented Dec 14, 2020

@Issif okay if you see so, but just to clarify my point of view, I meant that from falco side if we enable the http output (for falcosidekick or any other server) we may set unreachable url by mistake (I did that :() we can see that in falco logs but I thought it's better to introduce it as readiness probe to save debugging time and we read values from values.yaml like following:

 {{- if .Values.httpOutput.enabled }}
   readinessProbe:
      tcpSocket:
        host: {{ .Values.httpOutput.host }}
        port: {{ .Values.httpOutput.port }}
{{- end }}

I understand your point, and I agree the current solution needs to be improved, but I see an issue using the K8s probes. Falco could be restarted if the probe failed, and I don't believe that would be good. However, I still think we have to find a better way to inform the user when an output channel is misconfigured or is not working.
Btw, I had opened falcosecurity/falco#1466 about this topic.

@AbirHamzi
Copy link

@leogr yes I see what you mean, one last thing is it possible to keep the readiness probe and we set its failureThreshold high enough to avoid falco restarting in case of probe failure

@leogr
Copy link
Member Author

leogr commented Dec 14, 2020

@leogr yes I see what you mean, one last thing is it possible to keep the readiness probe and we set its failureThreshold high enough to avoid falco restarting in case of probe failure

It's hard to assume a default value for that a priori, IMHO. I'd rather leave open the ability to configure custom probes so that users may choose on a case-by-case basis.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants