Bug 1988351: OCM controller - periodically pull the data and update corresponding #375

tremes · 2021-03-16T09:15:14Z

secret

This adds a new OCM controller which periodically (each 8 hours) tries to pull the SCA (Simple content access) data from the OCM API. The SCA data (basically a x509 certificate) is exposed in the etc-pki-entitlement secret in the openshift-config-managed namespace.

Sample archive

No new data

Documentation

No update here

Unit Tests

Basic test covered in pkg/ocm/ocm_test.go

Privacy

Yes. There are no sensitive data in the newly collected information.

Changelog

References

https://issues.redhat.com/browse/CCXDEV-4211

openshift-ci-robot · 2021-03-16T09:15:26Z

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

openshift-ci-robot · 2021-03-16T09:15:29Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: tremes

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [tremes]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

pkg/ocm/ocm.go

pkg/insights/insightsclient/insightsclient.go

tremes · 2021-04-08T10:04:51Z

/test insights-operator-e2e-tests

tremes · 2021-06-25T10:29:34Z

/test insights-operator-e2e-tests

pkg/insights/insightsclient/insightsclient.go

natiiix

I don't see any serious issue here, so if this is the behaviour we want, then I think it's alright.

pkg/insights/insightsclient/insightsclient.go

tremes · 2021-07-22T10:03:50Z

~~Ohh I need to fix permissions for the secret in the openshift-config-managed namespace.~~ Done

mfojtik · 2021-07-26T09:02:08Z

pkg/config/config.go

@@ -24,6 +24,11 @@ type Serialized struct {
 	Impersonate             string   `json:"impersonate"`
 	Gather                  []string `json:"gather"`
 	EnableGlobalObfuscation bool     `json:"enableGlobalObfuscation"`
+	Ocm                     struct {


why is this defined as inline struct if you have this as full script on line 75? :-)

Ohh good catch! Thank you.

Well hmmm...this is slightly different type than the OCMConfig (which is embedded in Controller type). I can delete the OCMConfig type and define the attributes directly in the Controller type....I guess I don't have any better idea right now.

mfojtik · 2021-07-26T09:11:45Z

pkg/ocm/ocm.go

+
+const (
+	targetNamespaceName = "openshift-config-managed"
+	secretName          = "etc-pki-entitlement" //nolint: gosec


is this operator owning this secret? what operator manage this?

Yes ...I am not sure how the "ownership" is defined, but this secret is created/updated by the Insights Operator.

natiiix

Obligatory "I have not tested it locally", but I see nothing objectively wrong with the code and the tests look good to me. I mostly looked at the changes since my last review and it all makes sense to me, as far as I'm aware of the relevant task.

secret

…amespace

openshift-ci · 2021-07-30T11:16:22Z

@tremes: This pull request references Bugzilla bug 1988351, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.9.0) matches configured target release for branch (4.9.0)
bug is in the state NEW, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

No GitHub users were found matching the public email listed for the QA contact in Bugzilla (dmisharo@redhat.com), skipping review request.

In response to this:

Bug 1988351: OCM controller - periodically pull the data and update corresponding

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Serhii1011010

looks great to me

natiiix

I have reviewed this before and it should now be ready to be merged, so I'm giving it the real approval now. I have not tested this locally, or at least not recently, but I have checked the code again today and I see nothing worth arguing about.
/lgtm

openshift-ci · 2021-08-02T14:56:22Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: natiiix, Sergey1011010, tremes

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [Sergey1011010,natiiix,tremes]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

openshift-bot · 2021-08-02T15:08:04Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2021-08-02T18:34:58Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2021-08-02T19:39:58Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-ci · 2021-08-02T20:48:59Z

@tremes: All pull requests linked via external trackers have merged:

openshift/insights-operator#375

Bugzilla bug 1988351 has been moved to the MODIFIED state.

In response to this:

Bug 1988351: OCM controller - periodically pull the data and update corresponding

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 16, 2021

openshift-ci-robot requested review from 0sewa0 and mfojtik March 16, 2021 09:15

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 16, 2021

Serhii1011010 reviewed Mar 16, 2021

View reviewed changes

pkg/ocm/ocm.go Outdated Show resolved Hide resolved

tremes commented Mar 16, 2021

View reviewed changes

pkg/ocm/ocm.go Outdated Show resolved Hide resolved

natiiix reviewed Mar 16, 2021

View reviewed changes

pkg/ocm/ocm.go Outdated Show resolved Hide resolved

tremes force-pushed the ocm branch from e827081 to 1d88336 Compare April 8, 2021 10:03

tremes commented Apr 8, 2021

View reviewed changes

pkg/insights/insightsclient/insightsclient.go Outdated Show resolved Hide resolved

openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 11, 2021

tremes force-pushed the ocm branch from 8a28e81 to 28436d2 Compare April 12, 2021 06:41

openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 12, 2021

tremes force-pushed the ocm branch from 28436d2 to caf1667 Compare June 18, 2021 10:26

natiiix reviewed Jun 28, 2021

View reviewed changes

pkg/insights/insightsclient/insightsclient.go Outdated Show resolved Hide resolved

natiiix reviewed Jun 28, 2021

View reviewed changes

pkg/insights/insightsclient/insightsclient.go Outdated Show resolved Hide resolved

natiiix reviewed Jun 28, 2021

View reviewed changes

pkg/insights/insightsclient/insightsclient.go Outdated Show resolved Hide resolved

natiiix reviewed Jun 28, 2021

View reviewed changes

tremes mentioned this pull request Jun 30, 2021

fix obfuscation translation table secret manifest #466

Merged

4 tasks

tremes force-pushed the ocm branch from 737ad7a to af2c125 Compare June 30, 2021 12:02

tremes force-pushed the ocm branch from af2c125 to 7bc39e9 Compare July 22, 2021 07:47

tremes changed the title ~~WIP OCM controller - periodically pull the data and update corresponding~~ OCM controller - periodically pull the data and update corresponding Jul 22, 2021

tremes marked this pull request as ready for review July 22, 2021 07:50

openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 22, 2021

tremes commented Jul 22, 2021

View reviewed changes

pkg/insights/insightsclient/insightsclient.go Outdated Show resolved Hide resolved

mfojtik reviewed Jul 26, 2021

View reviewed changes

natiiix reviewed Jul 27, 2021

View reviewed changes

tremes added 9 commits July 30, 2021 09:07

OCM controller - periodically pull the data and update corresponding

cdc7c72

secret

Rename secret

9234d73

Rebase

3310096

Update & addd config attribute to disable the regular pulling

15d58bb

Add basic tests

01d8c13

Update the OCM communication PoC according the provided mock API

2203436

Fix the order of params in the insightsclient

884dbce

Add a new role to manage the secret in the openshift-config-managed n…

e865c86

…amespace

Add feature check

154641a

tremes force-pushed the ocm branch from 77bf120 to 154641a Compare July 30, 2021 08:43

tremes changed the title ~~OCM controller - periodically pull the data and update corresponding~~ Bug 1988351: OCM controller - periodically pull the data and update corresponding Jul 30, 2021

openshift-ci bot added bugzilla/severity-high Referenced Bugzilla bug's severity is high for the branch this PR is targeting. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. labels Jul 30, 2021

Serhii1011010 approved these changes Aug 2, 2021

View reviewed changes

natiiix approved these changes Aug 2, 2021

View reviewed changes

openshift-ci bot assigned natiiix Aug 2, 2021

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Aug 2, 2021

openshift-ci bot merged commit 54354a4 into openshift:master Aug 2, 2021

bparees mentioned this pull request Aug 5, 2021

Bug 1988308: Add feature for Insights operator pulling SCA certs openshift/api#983

Merged

tremes deleted the ocm branch November 26, 2021 08:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bug 1988351: OCM controller - periodically pull the data and update corresponding #375

Bug 1988351: OCM controller - periodically pull the data and update corresponding #375

tremes commented Mar 16, 2021 •

edited

Loading

openshift-ci-robot commented Mar 16, 2021

openshift-ci-robot commented Mar 16, 2021

tremes commented Apr 8, 2021

tremes commented Jun 25, 2021

natiiix left a comment

tremes commented Jul 22, 2021 •

edited

Loading

mfojtik Jul 26, 2021

tremes Jul 26, 2021

tremes Jul 26, 2021

mfojtik Jul 26, 2021

tremes Jul 26, 2021

natiiix left a comment •

edited

Loading

openshift-ci bot commented Jul 30, 2021

Serhii1011010 left a comment

natiiix left a comment

openshift-ci bot commented Aug 2, 2021

openshift-bot commented Aug 2, 2021

openshift-bot commented Aug 2, 2021

openshift-bot commented Aug 2, 2021

openshift-ci bot commented Aug 2, 2021

Bug 1988351: OCM controller - periodically pull the data and update corresponding #375

Bug 1988351: OCM controller - periodically pull the data and update corresponding #375

Conversation

tremes commented Mar 16, 2021 • edited Loading

Categories

Sample archive

Documentation

Unit Tests

Privacy

Changelog

References

openshift-ci-robot commented Mar 16, 2021

openshift-ci-robot commented Mar 16, 2021

tremes commented Apr 8, 2021

tremes commented Jun 25, 2021

natiiix left a comment

Choose a reason for hiding this comment

tremes commented Jul 22, 2021 • edited Loading

mfojtik Jul 26, 2021

Choose a reason for hiding this comment

tremes Jul 26, 2021

Choose a reason for hiding this comment

tremes Jul 26, 2021

Choose a reason for hiding this comment

mfojtik Jul 26, 2021

Choose a reason for hiding this comment

tremes Jul 26, 2021

Choose a reason for hiding this comment

natiiix left a comment • edited Loading

Choose a reason for hiding this comment

openshift-ci bot commented Jul 30, 2021

Serhii1011010 left a comment

Choose a reason for hiding this comment

natiiix left a comment

Choose a reason for hiding this comment

openshift-ci bot commented Aug 2, 2021

openshift-bot commented Aug 2, 2021

openshift-bot commented Aug 2, 2021

openshift-bot commented Aug 2, 2021

openshift-ci bot commented Aug 2, 2021

tremes commented Mar 16, 2021 •

edited

Loading

tremes commented Jul 22, 2021 •

edited

Loading

natiiix left a comment •

edited

Loading