Skip to content

ROX-9876: Add ID to internal image integration in Sensor#1073

Merged
RTann merged 6 commits intomasterfrom
ROX-9876
Mar 25, 2022
Merged

ROX-9876: Add ID to internal image integration in Sensor#1073
RTann merged 6 commits intomasterfrom
ROX-9876

Conversation

@RTann
Copy link
Copy Markdown
Contributor

@RTann RTann commented Mar 25, 2022

Description

Image Integrations in Sensor lacked an ID, so it was not indexed properly. Unit tests for the secret listener missed this.

Use the registry's name as the ID because that is how it is indexed in the dockercfg secret.

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
  • [ ] Documented user facing changes (create PR based on stackrox/openshift-docs and merge into rhacs-docs)

Testing Performed

CI. Without this fix, these tests fail.

@ghost
Copy link
Copy Markdown

ghost commented Mar 25, 2022

Tag for build #353525 is 3.69.x-213-g8a7f46a650.

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

export MAIN_IMAGE_TAG='3.69.x-213-g8a7f46a650'

📦 You can also generate an installation bundle with:

docker run -i --rm stackrox/main:3.69.x-213-g8a7f46a650 central generate interactive > bundle.zip

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

@parametalol parametalol added this to the 69.1-rc.3 milestone Mar 25, 2022
Copy link
Copy Markdown
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.

Approving to not block the merge as the change works.
Please check if it is possible to avoid this in the data layer (also okay to defer to another PR imho).


regs := rs.getRegistries(namespace)
err = regs.UpdateImageIntegration(&storage.ImageIntegration{
Id: registry,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I recommend letting UpdateImageIntegration set the registry as the ID automatically or validating it before persisting to avoid saving corrupted integrations.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is called in other places where the ID is already populated as a UUID. This is a case where we dont want to use a UUID because of how it's used. I think this is ok

@RTann RTann enabled auto-merge (squash) March 25, 2022 15:03
@RTann RTann merged commit 7193fba into master Mar 25, 2022
@RTann RTann deleted the ROX-9876 branch March 25, 2022 16:05
parametalol pushed a commit that referenced this pull request Mar 28, 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.

3 participants