Skip to content

ROX-9127: Create CertificateRequester to request certificates from central#455

Closed
juanrh wants to merge 6 commits intomasterfrom
juanrh/ROX-9127
Closed

ROX-9127: Create CertificateRequester to request certificates from central#455
juanrh wants to merge 6 commits intomasterfrom
juanrh/ROX-9127

Conversation

@juanrh
Copy link
Contributor

@juanrh juanrh commented Jan 26, 2022

Description

The CertificateRequester to request certificates from central using two channels that can be provided by a SensorComponent, see example usage in draft PR #400

Checklist

  • Investigated and inspected CI test results
  • Unit test and regression tests added
  • Evaluated and added CHANGELOG entry if required
  • Determined and documented upgrade steps

No CHANGELOG entry or upgrade steps required.

Testing Performed

Added unit test

@ghost
Copy link

ghost commented Jan 26, 2022

Tag for build #147884 is 3.68.x-47-g573bd2887c.

💻 For deploying this image using the dev scripts, run the following first:

export MAIN_IMAGE_TAG='3.68.x-47-g573bd2887c'

📦 You can also generate an installation bundle with:

docker run -i --rm stackrox/main:3.68.x-47-g573bd2887c central generate interactive > bundle.zip

🕹️ A roxctl binary artifact can be downloaded from CircleCI.

Copy link
Contributor

@SimonBaeumer SimonBaeumer left a comment

Choose a reason for hiding this comment

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

Your PR turn out to be a good async go training, I really enjoy it! Keep up the good work!

As said in a comment this approach does not work if multiple requests are pending. If this happens this could cause the pod running out of memory because on each cert refresh tick a new gorutine is spawned which may never receive the request.

To dispatch the response to the corresponding request an index may needed here.

@juanrh
Copy link
Contributor Author

juanrh commented Jan 27, 2022

As said in a comment this approach does not work if multiple requests are pending. If this happens this could cause the pod running out of memory because on each cert refresh tick a new gorutine is spawned which may never receive the request.

RequestCertificates is blocking and cancellable with its context parameter, so it would not run forever, and also the ticker only runs 1 call to the ticker function at a time, so I don't see how this could cause an out of memory error. In any case, as I'll going the dispatcher route as suggested, see specific comment for details

@juanrh juanrh closed this Jan 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants