NO-ISSUE: Deprecate and disable support for OCM offline token #6307

jhernand · 2024-05-11T10:26:22Z

Currently or ocm/client package supports authentication to OCM using an offline token via the OCM_SELF_TOKEN environment variable. But the OCM team has deprecated this authentication mechanism and will remove it in the future. The alternative is to use a client identifier and client secret, which is what we use in the SaaS environment. This patch disables by default that support, and will make the server fail with an explicit error message if it is used. Users that really need to use it will need to explicitly enable it setting the ACKNOWLEDGE_DEPRECATED_OCM_SELF_TOKEN environment variable to yes. In that case the server will start, but a warning will be written to the log.

List all the issues related to this PR

What environments does this code impact?

Automation (CI, tools, etc)
Cloud
Operator Managed Deployments
None

How was this code tested?

assisted-test-infra environment
dev-scripts environment
Reviewer's test appreciated
Waiting for CI to do a full test run
Manual (Elaborate on how it was tested)
No tests needed

Checklist

Title and description added to both, commit and PR.
Relevant issues have been associated (see CONTRIBUTING guide)
This change does not require a documentation update (docstring, docs, README, etc)
Does this change include unit-tests (note that code changes require unit-tests)

Reviewers Checklist

Are the title and description (in both PR and commit) meaningful and clear?
Is there a bug required (and linked) for this change?
Should this PR be backported?

openshift-ci-robot · 2024-05-11T10:26:25Z

@jhernand: This pull request explicitly references no jira issue.

In response to this:

Currently or ocm/client package supports authentication to OCM using an offline token via the OCM_SELF_TOKEN environment variable. But the OCM team has deprecated this authentication mechanism and will remove it in the future. The alternative is to use a client identifier and client secret, which is what we use in the SaaS environment. This patch disables by default that support, and will make the server fail with an explicit error message if it is used. Users that really need to use it will need to explicitly enable it setting the ACKNOWLEDGE_DEPRECATED_OCM_SELF_TOKEN environment variable to yes. In that case the server will start, but a warning will be written to the log.

List all the issues related to this PR

New Feature

Enhancement

Bug fix

Tests

Documentation

CI/CD

What environments does this code impact?

Automation (CI, tools, etc)

Cloud

Operator Managed Deployments

None

How was this code tested?

assisted-test-infra environment

dev-scripts environment

Reviewer's test appreciated

Waiting for CI to do a full test run

Manual (Elaborate on how it was tested)

No tests needed

Checklist

Title and description added to both, commit and PR.

Relevant issues have been associated (see CONTRIBUTING guide)

This change does not require a documentation update (docstring, docs, README, etc)

Does this change include unit-tests (note that code changes require unit-tests)

Reviewers Checklist

Are the title and description (in both PR and commit) meaningful and clear?

Is there a bug required (and linked) for this change?

Should this PR be backported?

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 openshift-eng/jira-lifecycle-plugin repository.

openshift-ci · 2024-05-11T10:26:55Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jhernand

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~~ [jhernand]

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

Currently or `ocm/client` package supports authentication to OCM using an offline token via the `OCM_SELF_TOKEN` environment variable. But the OCM team has deprecated this authentication mechanism and will remove it in the future. The alternative is to use a client identifier and client secret, which is what we use in the SaaS environment. This patch disables by default that support, and will make the server fail with an explicit error message if it is used. Users that really need to use it will need to explicitly enable it setting the `ACKNOWLEDGE_DEPRECATED_OCM_SELF_TOKEN` environment variable to `yes`. In that case the server will start, but a warning will be written to the log. Signed-off-by: Juan Hernandez <juan.hernandez@redhat.com>

codecov · 2024-05-11T10:44:00Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 68.98%. Comparing base (904ee71) to head (0dcfead).
Report is 17 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #6307      +/-   ##
==========================================
+ Coverage   68.29%   68.98%   +0.69%     
==========================================
  Files         241      245       +4     
  Lines       35863    39450    +3587     
==========================================
+ Hits        24491    27213    +2722     
- Misses       9212     9813     +601     
- Partials     2160     2424     +264

see 20 files with indirect coverage changes

rccrdpccl · 2024-05-14T08:22:24Z

/retest

gamli75 · 2024-05-23T06:40:11Z

/test edge-e2e-metal-assisted

eifrach · 2024-05-27T07:02:15Z

/lgtm

openshift-ci-robot · 2024-05-27T08:58:28Z

/retest-required

Remaining retests: 0 against base HEAD 4f4c464 and 2 for PR HEAD 0dcfead in total

rccrdpccl · 2024-05-27T09:03:10Z

/retest

rccrdpccl · 2024-05-27T09:04:22Z

pkg/ocm/client.go

@@ -113,7 +125,25 @@ func (c *Client) newConnection() error {
 	if c.Config.ClientID != "" && c.Config.ClientSecret != "" {
 		builder = builder.Client(c.Config.ClientID, c.Config.ClientSecret)
 	} else if c.Config.SelfToken != "" {
-		builder = builder.Tokens(c.Config.SelfToken)
+		if strings.EqualFold(c.Config.AcknowledgeDeprecatedSelfToken, "yes") {


nit: if we invert this condition we can exit early and remove else condition

openshift-ci-robot · 2024-05-27T11:51:33Z

/retest-required

Remaining retests: 0 against base HEAD 8a63485 and 1 for PR HEAD 0dcfead in total

openshift-ci · 2024-05-27T18:25:31Z

@jhernand: all tests passed!

Full PR test history. Your PR dashboard.

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-sigs/prow repository. I understand the commands that are listed here.

openshift-bot · 2024-05-27T21:00:27Z

[ART PR BUILD NOTIFIER]

This PR has been included in build ose-agent-installer-api-server-container-v4.17.0-202405272012.p0.g5b71872.assembly.stream.el9 for distgit ose-agent-installer-api-server.
All builds following this will include this PR.

openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label May 11, 2024

openshift-ci bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label May 11, 2024

openshift-ci bot requested review from danielerez and eifrach May 11, 2024 10:26

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 11, 2024

jhernand force-pushed the deprecate_and_disable_use_of_ocm_offline_token branch from b49124d to efbe9f9 Compare May 11, 2024 10:30

jhernand force-pushed the deprecate_and_disable_use_of_ocm_offline_token branch from efbe9f9 to 0dcfead Compare May 11, 2024 10:30

openshift-ci bot assigned eifrach May 27, 2024

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label May 27, 2024

rccrdpccl reviewed May 27, 2024

View reviewed changes

openshift-merge-bot bot merged commit 5b71872 into openshift:master May 27, 2024
14 checks passed

jhernand deleted the deprecate_and_disable_use_of_ocm_offline_token branch May 28, 2024 07:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

NO-ISSUE: Deprecate and disable support for OCM offline token #6307

NO-ISSUE: Deprecate and disable support for OCM offline token #6307

jhernand commented May 11, 2024

openshift-ci-robot commented May 11, 2024

List all the issues related to this PR

What environments does this code impact?

How was this code tested?

Checklist

Reviewers Checklist

openshift-ci bot commented May 11, 2024

codecov bot commented May 11, 2024 •

edited

rccrdpccl commented May 14, 2024

gamli75 commented May 23, 2024

eifrach commented May 27, 2024

openshift-ci-robot commented May 27, 2024

rccrdpccl commented May 27, 2024

rccrdpccl May 27, 2024

openshift-ci-robot commented May 27, 2024

openshift-ci bot commented May 27, 2024

openshift-bot commented May 27, 2024

NO-ISSUE: Deprecate and disable support for OCM offline token #6307

NO-ISSUE: Deprecate and disable support for OCM offline token #6307

Conversation

jhernand commented May 11, 2024

List all the issues related to this PR

What environments does this code impact?

How was this code tested?

Checklist

Reviewers Checklist

openshift-ci-robot commented May 11, 2024

List all the issues related to this PR

What environments does this code impact?

How was this code tested?

Checklist

Reviewers Checklist

openshift-ci bot commented May 11, 2024

codecov bot commented May 11, 2024 • edited

Codecov Report

rccrdpccl commented May 14, 2024

gamli75 commented May 23, 2024

eifrach commented May 27, 2024

openshift-ci-robot commented May 27, 2024

rccrdpccl commented May 27, 2024

rccrdpccl May 27, 2024

Choose a reason for hiding this comment

openshift-ci-robot commented May 27, 2024

openshift-ci bot commented May 27, 2024

openshift-bot commented May 27, 2024

codecov bot commented May 11, 2024 •

edited