Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

Images are ready for the commit at 690cae2.

To use with deploy scripts, first export MAIN_IMAGE_TAG=4.7.x-479-g690cae2e76.

Codecov Report

Attention: Patch coverage is 23.72881% with 90 lines in your changes missing coverage. Please review.

Project coverage is 48.56%. Comparing base (d5b309b) to head (e79a523).
Report is 44 commits behind head on master.

@@            Coverage Diff             @@
##           master   #13239      +/-   ##
==========================================
+ Coverage   48.54%   48.56%   +0.01%     
==========================================
  Files        2467     2474       +7     
  Lines      177811   178837    +1026     
==========================================
+ Hits        86319    86849     +530     
- Misses      84564    85045     +481     
- Partials     6928     6943      +15     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚨 Try these New Features:

• Flaky Tests Detection - Detect and resolve failed and flaky tests
• JS Bundle Analysis - Avoid shipping oversized bundles

/retest

/test all

FTR, the updated code, including the refactoring of the crs flow implementation, is tested successfully in a follow-up PR.

/retest

Finished a first pass. It looks good. I have a couple of overall questions:

• Out of curiosity, are these init containers executed every time the pod restarts or only the first time?
• Do I need to rebase any branches in order to test these changes? I'd like to play a little bit with different sensor/central versions.

The init containers are executed on every pod start. But the CRS container would terminate very quickly, because it sees that nothing is to do and the init-tls-certs container should be very quick as well, it's a custom go binary that just does a filesystem lookup and copies to an emptyDir.

• Do I need to rebase any branches in order to test these changes? I'd like to play a little bit with different sensor/central versions.

Yes, you can test this. Go to this branch for that. It contains the changes of this branch plus the necessary modifications to the Helm chart.

@mclasmeier I was able to make sensor panic by:

1. Deploying a central that supports CRS installations
2. Generating the helm charts from ROX-25944: CRS Support in Helm Chart #12713 using roxctl
3. Generating an init-bundle
4. Deploying sensor with:
   • Image tag: 4.6.1
   • ROX_DEPLOY_SENSOR_WITH_CRS set to true
   • Using the helm charts generated in step (2)

I'm not sure if this is something that we even want to address. It looks to me like a very extreme case but it thought it was worth mentioning.

@mclasmeier I was able to make sensor panic by:

1. Deploying a central that supports CRS installations

2. Generating the helm charts from ROX-25944: CRS Support in Helm Chart #12713 using roxctl

3. Generating an init-bundle

4. Deploying sensor with:

   • Image tag: 4.6.1
   • ROX_DEPLOY_SENSOR_WITH_CRS set to true
   • Using the helm charts generated in step (2)

I'm not sure if this is something that we even want to address. It looks to me like a very extreme case but it thought it was worth mentioning.

Thank you for the testing!

I think what you are outlining above seems like somewhat of an artificial breakage caused by combining code fragments from different versions that were not designed to play nice with each other. My guess is that in your scenario the following happens:

ROX_DEPLOY_SENSOR_WITH_CRS=true (within the deploy scripts) causes the new Helm chart to use CRS. But the specified sensor (4.6.1) doesn't know how to do CRS, hence sensor would immediately try to start normally as a service and fails to find certificates?

/retest

I think what you are outlining above seems like somewhat of an artificial breakage caused by combining code fragments from different versions that were not designed to play nice with each other.

Agree.

ROX_DEPLOY_SENSOR_WITH_CRS=true (within the deploy scripts) causes the new Helm chart to use CRS.

Just to clarify. I didn't use the deploy scripts. I generated the helm charts with roxctl v4.7 and I had the ROX_DEPLOY_SENSOR_WITH_CRS set to true in my terminal, but I deployed directly with helm.

hence sensor would immediately try to start normally as a service and fails to find certificates?

Yeah sensor panics because it cannot find the certificates. There's only one way I can see we can avoid this (if we want to):

• Patching old sensor to not panic if the certificates are missing.
• I don't know if it's possible but we could add a check in the helm charts that makes sure the CRS path is not taken if sensor version < 4.7 (?)

• Patching old sensor to not panic if the certificates are missing.

Isn't sensor deliberately panicking in such a case? In any case, it would still require an upgrade on the user's side to benefit from whatever we change here.

• I don't know if it's possible but we could add a check in the helm charts that makes sure the CRS path is not taken if sensor version < 4.7 (?)

Thank you, I think this one -- if feasible -- would be the way to go. But not as part of this PR.

Just to clarify. I didn't use the deploy scripts. I generated the helm charts with roxctl v4.7 and I had the ROX_DEPLOY_SENSOR_WITH_CRS set to true in my terminal, but I deployed directly with helm.

Hmm.
I think ROX_DEPLOY_SENSOR_WITH_CRS is only relevant for our deploy scripts, it's not consumed during generation of the Helm chart with roxctl? That's why I thought you were using the deploy scripts.

LGTM! Thank you for the changes!

Thank you so much!