Skip to content

Conversation

@krezovic
Copy link
Contributor

@krezovic krezovic commented May 5, 2025

Check beforehand if tmp dir contains the config file and try to re-use it

Fixes: #252

/kind enhancement
/area configuration

@krezovic krezovic marked this pull request as draft May 5, 2025 18:20
@krezovic krezovic force-pushed the feat/252 branch 3 times, most recently from 39b8f6d to af54fa4 Compare June 21, 2025 20:17
@krezovic krezovic marked this pull request as ready for review June 21, 2025 20:19
@krezovic krezovic force-pushed the feat/252 branch 2 times, most recently from 19c0640 to 32c5f72 Compare August 2, 2025 10:39
@SISheogorath
Copy link

While I do enjoy that you tackle the "Let's install packages on container startup" problem, I'm not sure the "let's allow the Pod to patch all Pods in the same namespace" is a good trade.

The solution of using an encrypted annotation to store the secret, seems overly complex to me.

Other approaches:

  • Use a named secret, that you can patch to store the secret, this prevents a compromised crowdsec Pod from pivoting to other Pods. (If you want to avoid being unable to start your pod, you can create the secret empty and patch the key/value in later.)
  • Use a helm hook to create and re-validate the api-key.
  • Use a cronjob to re-validate the api-key.

@krezovic
Copy link
Contributor Author

krezovic commented Dec 5, 2025

While I do enjoy that you tackle the "Let's install packages on container startup" problem, I'm not sure the "let's allow the Pod to patch all Pods in the same namespace" is a good trade.

The solution of using an encrypted annotation to store the secret, seems overly complex to me.

Other approaches:

  • Use a named secret, that you can patch to store the secret, this prevents a compromised crowdsec Pod from pivoting to other Pods. (If you want to avoid being unable to start your pod, you can create the secret empty and patch the key/value in later.)
  • Use a helm hook to create and re-validate the api-key.
  • Use a cronjob to re-validate the api-key.

Hello @SISheogorath. Thank you for your feedback.

I have considered using any additional resource but gave up on it due to the following:

  • We do not know the machine (POD) names beforehand, and they will change on every deploy. We would have to clear the secret somehow on every new deploy. Helm Hook may work.
  • There may be any number of PODs, dependent on the number of nodes for daemonsets, and number of desired replicas for deployments. Handling concurrent modification of a single resource from many instances on startup is no easy task. Additionally, in a niche scenario, there may be so many instances (improbable, but possible) where the secret will end up storing more than 1 MB of data, which is the default etcd limit.

I did some investigation to address your concern of a compromised POD and the solution may be to use projected volume for a service account only on init container, with minimal expiration time. That way, whole POD won't get the privileges to patch anything, but init container will, and those permissions will be revoked once init container is finished.

https://kubernetes.io/docs/tasks/configure-pod-container/configure-service-account/#serviceaccount-token-volume-projection

@SISheogorath
Copy link

If you want to store the token only for the lifetime of a Pod, it would be easier to use a (memory-backed) emptyDir to store the token.

https://kubernetes.io/docs/concepts/storage/volumes/#emptydir-memory-configuration-example

Why all the complications with annotating it to the Pod? There is no need to talk to the Kubernetes API if you just store it in an emptyDir that survives until the Node is restarts (memory-backed emptydir) or the pod is deleted (regular emptyDir).

@krezovic
Copy link
Contributor Author

krezovic commented Dec 7, 2025

If you want to store the token only for the lifetime of a Pod, it would be easier to use a (memory-backed) emptyDir to store the token.

https://kubernetes.io/docs/concepts/storage/volumes/#emptydir-memory-configuration-example

Why all the complications with annotating it to the Pod? There is no need to talk to the Kubernetes API if you just store it in an emptyDir that survives until the Node is restarts (memory-backed emptydir) or the pod is deleted (regular emptyDir).

Well this is embarrassing. POD actually uses tmpDir, I just had to check if the tmpdir was written to before attempting to register.

Thanks for the hint and feedback, the solution became much more simple :)

@krezovic
Copy link
Contributor Author

krezovic commented Dec 7, 2025

Hello @blotus and @sabban. This MR became much more simpler after addressing feedback from @SISheogorath. Would you please take a look and maybe finally merge it? Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

403 Forbidden on PODs after host reboot

2 participants