ROX-8758: Parameterize ALL scanner-related resource names.#558
ROX-8758: Parameterize ALL scanner-related resource names.#558
Conversation
|
Tag for build #177144 is 💻 For deploying this image using the dev scripts, run the following first: export MAIN_IMAGE_TAG='3.68.x-130-g963a1bde3a'📦 You can also generate an installation bundle with: docker run -i --rm stackrox/main:3.68.x-130-g963a1bde3a central generate interactive > bundle.zip🕹️ A |
The changes look good but I want to reiterate on adding different names to all resources after seeing it in code. Take the operator for example, if we start adding custom reconciliation to scanner (i.e. scaling) we need to support Given the advantage of deploying it into the same namespace (or cluster) for better (developer) UX. Instead we could do a lookup on an existing Lastly installing the same application two times in the same namespace is often not supported and a reasonable argument to not support it. What do you think @porridge? |
|
After watching @RTann 's scanner brown bag and chatting to him, I had the impression that talking to local scanner would be a different code path from talking to the global one. So it is not necessarily a problem that the scanner connection hostname would differ. But I might have gotten that wrong, so I'll let him comment on that. Regarding the cost/benefit trade-off: it seems to me that this change will actually make (e2e) testing simpler, since in a single-cluster scenarios we will be able to continue using a single namespace (it's particularly cumbersome to use more than one with |
| [< end >] | ||
|
|
||
| {{ if or (eq $scannerCfg.mode "") (eq $scannerCfg.mode "full") }} | ||
| {{ $_ := set $scannerCfg "name" "scanner" }} |
There was a problem hiding this comment.
@SimonBaeumer one thing I wanted to ask is if this should instead be:
| {{ $_ := set $scannerCfg "name" "scanner" }} | |
| {{ $_ := set $scannerCfg "_name" "scanner" }} |
since whether an identifier starts with an underscore seems random to me :-/
There was a problem hiding this comment.
To me this looks fine 👍
| .secrets["scanner-slim-tls"] | assertThat(. == null) | ||
| .secrets["scanner-slim-db-tls"] | assertThat(. == null) |
There was a problem hiding this comment.
@juanrh Does this affect your pull-requests? I think you need to create scanner-slim-db-tls and scanner-slim-tls secrets instead of scanner-tls
There was a problem hiding this comment.
Thanks for the heads up, I'll take that into account for #565
Description
Use the word
scannerwhen in full mode, andscanner-slimwhile in slim mode.This parameterisation applies the following:
scannerin theoverridesection of chart values. This is debatable, but I chose to be consistent with the actual resource names since this is what users likely expect when they tweak at such level of detail.Note that any uses of the above in application code will need parameterisation as well.
cc @RTann @juanrh
For the avoidance of doubt, there are NO CHANGES to:
scannerin thecustomizesection of chart values. See rationale above.scannerandscanner-dbin TLScertSpecs passed toconfigureCryptocalls. This might also be debatable, if we want these to match theServicenames.Checklist
Unit test and regression tests addedEvaluated and added CHANGELOG entry if requiredDetermined and documented upgrade stepsIf any of these don't apply, please comment below.
Testing Performed