OSD-28241: Allow building backplane config, login with provided OCM connections

[nephomaniac]

What type of PR is this?

• Bug
• Feature
• Documentation
• Test Coverage
• Clean Up
• Others

What this PR does / Why we need it?

This PR allows an OCM connection to be provided to the config building and login functions. ie adding new '*WithConn()' function variants allowing the caller to provide the OCM connection. This PR attempts to allow utils sharing backplane code to interact with clusters which reside in different OCM envs within the same executable. The most relevant use case is testing/developing within pre-production STAGE and INTEGRATION OCM envs, that are serviced by Hive clusters which exist in OCM Production environment(s)

• Today as part of the client connection setup/code-path, it is not possible to create a clean separation of OCM config to be provided to multiple backplane connections within a single executable. It appears that through a chain of functions, the use of env vars prevents this from working as desired.

• These changes also help avoid rebuilding/tearing down multiple OCM connections during the config + login phases.

• This may make the build/login path more consistent with regard to the OCM attributes they use. Rebuilding/refetching OCM attributes may introduce (small) windows for race conditions.

Which Jira/Github issue(s) does this PR fix?

• Related Issue # OSD-28241,
  SREP-164, SREP-183
• Closes #

Special notes for your reviewer

After initial draft review, additional unit tests should be added.

Unit Test Coverage

Waiting for initial draft review, before adding remaining unit tests.
I've manually tested this branch against OSDCTL using both legacy backplane functions, and new OCM sdk connection fed backplane functions. I've connected to clusters using each path, as well as mixing these within a single util. I've also tested basic ocm-backplane login cli connections in the terminal.

Guidelines

• If it's a new sub-command or new function to an existing sub-command, please cover at least 50% of the code
• If it's a bug fix for an existing sub-command, please cover 70% of the code

Test coverage checks

• Added unit tests
• Created jira card to add unit test
• This PR may not need unit tests

Pre-checks (if applicable)

• Ran unit tests locally
• Validated the changes in a cluster
• Included documentation changes with PR

[openshift-ci-robot]

@nephomaniac: This pull request references OSD-28241 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.19.0" version, but no target version was set.

In response to this:

What type of PR is this?

• Bug
• Feature
• Documentation
• Test Coverage
• Clean Up
• [ X] Others

What this PR does / Why we need it?

This PR allows an OCM connection to be provided to the config building and login functions. This PR attempts to allow utils sharing backplane code to interact with clusters which reside in different OCM envs within the same executable. The most relevant use case is testing/developing within pre-production STAGE and INTEGRATION OCM envs, that are serviced by Hive clusters which exist in OCM Production environment(s)

• Today as part of the client connection setup/code-path, it is not possible to create a clean separation of OCM config to be provided to multiple backplane connections within a single executable. It appears that through a chain of functions, the use of env vars prevents this from working as desired.

• These changes also help avoid rebuilding/tearing down multiple OCM connections during the config + login phases.

• This may make the build/login path more consistent with regard to the OCM attributes they use. Rebuilding/refetching OCM attributes may introduce (small) windows for race conditions.

Which Jira/Github issue(s) does this PR fix?

• Related Issue # OSD-28241
• Closes #

Special notes for your reviewer

After initial draft review, additional unit tests should be added.

Unit Test Coverage

Waiting for initial draft review, before adding remaining unit tests.
I've manually tested this branch against OSDCTL using both legacy backplane functions, and new OCM sdk connection fed backplane functions. I've connected to clusters using each path, as well as mixing these within a single util. I've also tested basic ocm-backplane login cli connections in the terminal.

Guidelines

• If it's a new sub-command or new function to an existing sub-command, please cover at least 50% of the code
• If it's a bug fix for an existing sub-command, please cover 70% of the code

Test coverage checks

• Added unit tests
• Created jira card to add unit test
• This PR may not need unit tests

Pre-checks (if applicable)

• Ran unit tests locally
• Validated the changes in a cluster
• Included documentation changes with PR

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]

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]

@nephomaniac: This pull request references OSD-28241 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.19.0" version, but no target version was set.

In response to this:

What type of PR is this?

• Bug
• Feature
• Documentation
• Test Coverage
• Clean Up
• [ X] Others

What this PR does / Why we need it?

This PR allows an OCM connection to be provided to the config building and login functions. This PR attempts to allow utils sharing backplane code to interact with clusters which reside in different OCM envs within the same executable. The most relevant use case is testing/developing within pre-production STAGE and INTEGRATION OCM envs, that are serviced by Hive clusters which exist in OCM Production environment(s)

• Today as part of the client connection setup/code-path, it is not possible to create a clean separation of OCM config to be provided to multiple backplane connections within a single executable. It appears that through a chain of functions, the use of env vars prevents this from working as desired.

• These changes also help avoid rebuilding/tearing down multiple OCM connections during the config + login phases.

• This may make the build/login path more consistent with regard to the OCM attributes they use. Rebuilding/refetching OCM attributes may introduce (small) windows for race conditions.

Which Jira/Github issue(s) does this PR fix?

• Related Issue # OSD-28241
• Closes #

Special notes for your reviewer

After initial draft review, additional unit tests should be added.

Unit Test Coverage

Waiting for initial draft review, before adding remaining unit tests.
I've manually tested this branch against OSDCTL using both legacy backplane functions, and new OCM sdk connection fed backplane functions. I've connected to clusters using each path, as well as mixing these within a single util. I've also tested basic ocm-backplane login cli connections in the terminal.

Guidelines

• If it's a new sub-command or new function to an existing sub-command, please cover at least 50% of the code
• If it's a bug fix for an existing sub-command, please cover 70% of the code

Test coverage checks

• Added unit tests
• Created jira card to add unit test
• This PR may not need unit tests

Pre-checks (if applicable)

• Ran unit tests locally
• Validated the changes in a cluster
• Included documentation changes with PR

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-robot]

@nephomaniac: This pull request references OSD-28241 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.19.0" version, but no target version was set.

In response to this:

What type of PR is this?

• Bug
• Feature
• Documentation
• Test Coverage
• Clean Up
• Others

What this PR does / Why we need it?

This PR allows an OCM connection to be provided to the config building and login functions. This PR attempts to allow utils sharing backplane code to interact with clusters which reside in different OCM envs within the same executable. The most relevant use case is testing/developing within pre-production STAGE and INTEGRATION OCM envs, that are serviced by Hive clusters which exist in OCM Production environment(s)

• Today as part of the client connection setup/code-path, it is not possible to create a clean separation of OCM config to be provided to multiple backplane connections within a single executable. It appears that through a chain of functions, the use of env vars prevents this from working as desired.

• These changes also help avoid rebuilding/tearing down multiple OCM connections during the config + login phases.

• This may make the build/login path more consistent with regard to the OCM attributes they use. Rebuilding/refetching OCM attributes may introduce (small) windows for race conditions.

Which Jira/Github issue(s) does this PR fix?

• Related Issue # OSD-28241
• Closes #

Special notes for your reviewer

After initial draft review, additional unit tests should be added.

Unit Test Coverage

Waiting for initial draft review, before adding remaining unit tests.
I've manually tested this branch against OSDCTL using both legacy backplane functions, and new OCM sdk connection fed backplane functions. I've connected to clusters using each path, as well as mixing these within a single util. I've also tested basic ocm-backplane login cli connections in the terminal.

Guidelines

• If it's a new sub-command or new function to an existing sub-command, please cover at least 50% of the code
• If it's a bug fix for an existing sub-command, please cover 70% of the code

Test coverage checks

• Added unit tests
• Created jira card to add unit test
• This PR may not need unit tests

Pre-checks (if applicable)

• Ran unit tests locally
• Validated the changes in a cluster
• Included documentation changes with PR

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]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: nephomaniac
Once this PR has been reviewed and has the lgtm label, please assign hectorakemp for approval. For more information see the Code Review Process.

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

Needs approval from an approver in each of these files:

• OWNERS

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

[nephomaniac]

/test all

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 21.62162% with 58 lines in your changes missing coverage. Please review.

Project coverage is 47.36%. Comparing base (4ca1dd8) to head (dd35199).
Report is 24 commits behind head on main.

Files with missing lines	Patch %	Lines
cmd/ocm-backplane/login/login.go	25.00%	13 Missing and 2 partials ⚠️
pkg/cli/config/config.go	45.83%	12 Missing and 1 partial ⚠️
pkg/backplaneapi/mocks/clientUtilsMock.go	0.00%	9 Missing ⚠️
pkg/ocm/mocks/ocmWrapperMock.go	0.00%	9 Missing ⚠️
pkg/backplaneapi/clientUtils.go	0.00%	8 Missing ⚠️
pkg/ocm/ocm.go	0.00%	4 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #657      +/-   ##
==========================================
- Coverage   47.55%   47.36%   -0.20%     
==========================================
  Files          90       90              
  Lines        7068     7257     +189     
==========================================
+ Hits         3361     3437      +76     
- Misses       3299     3405     +106     
- Partials      408      415       +7     

Files with missing lines	Coverage Δ
pkg/ocm/ocm.go	0.00% <0.00%> (ø)
pkg/backplaneapi/clientUtils.go	0.00% <0.00%> (ø)
pkg/backplaneapi/mocks/clientUtilsMock.go	60.29% <0.00%> (-9.20%)	⬇️
pkg/ocm/mocks/ocmWrapperMock.go	80.11% <0.00%> (-4.20%)	⬇️
pkg/cli/config/config.go	72.67% <45.83%> (-3.69%)	⬇️
cmd/ocm-backplane/login/login.go	66.50% <25.00%> (-2.09%)	⬇️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[nephomaniac]

/test lint

[nephomaniac]

/test all

[openshift-ci]

@nephomaniac: 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-ci-robot]

@nephomaniac: This pull request references OSD-28241 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.20.0" version, but no target version was set.

In response to this:

What type of PR is this?

• Bug
• Feature
• Documentation
• Test Coverage
• Clean Up
• Others

What this PR does / Why we need it?

This PR allows an OCM connection to be provided to the config building and login functions. This PR attempts to allow utils sharing backplane code to interact with clusters which reside in different OCM envs within the same executable. The most relevant use case is testing/developing within pre-production STAGE and INTEGRATION OCM envs, that are serviced by Hive clusters which exist in OCM Production environment(s)

• Today as part of the client connection setup/code-path, it is not possible to create a clean separation of OCM config to be provided to multiple backplane connections within a single executable. It appears that through a chain of functions, the use of env vars prevents this from working as desired.

• These changes also help avoid rebuilding/tearing down multiple OCM connections during the config + login phases.

• This may make the build/login path more consistent with regard to the OCM attributes they use. Rebuilding/refetching OCM attributes may introduce (small) windows for race conditions.

Which Jira/Github issue(s) does this PR fix?

• Related Issue # OSD-28241, SREP-164(https://issues.redhat.com/browse/SREP-164), SREP-183
• Closes #

Special notes for your reviewer

After initial draft review, additional unit tests should be added.

Unit Test Coverage

Waiting for initial draft review, before adding remaining unit tests.
I've manually tested this branch against OSDCTL using both legacy backplane functions, and new OCM sdk connection fed backplane functions. I've connected to clusters using each path, as well as mixing these within a single util. I've also tested basic ocm-backplane login cli connections in the terminal.

Guidelines

• If it's a new sub-command or new function to an existing sub-command, please cover at least 50% of the code
• If it's a bug fix for an existing sub-command, please cover 70% of the code

Test coverage checks

• Added unit tests
• Created jira card to add unit test
• This PR may not need unit tests

Pre-checks (if applicable)

• Ran unit tests locally
• Validated the changes in a cluster
• Included documentation changes with PR

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-robot]

@nephomaniac: This pull request references OSD-28241 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.20.0" version, but no target version was set.

In response to this:

What type of PR is this?

• Bug
• Feature
• Documentation
• Test Coverage
• Clean Up
• Others

What this PR does / Why we need it?

This PR allows an OCM connection to be provided to the config building and login functions. This PR attempts to allow utils sharing backplane code to interact with clusters which reside in different OCM envs within the same executable. The most relevant use case is testing/developing within pre-production STAGE and INTEGRATION OCM envs, that are serviced by Hive clusters which exist in OCM Production environment(s)

• Today as part of the client connection setup/code-path, it is not possible to create a clean separation of OCM config to be provided to multiple backplane connections within a single executable. It appears that through a chain of functions, the use of env vars prevents this from working as desired.

• These changes also help avoid rebuilding/tearing down multiple OCM connections during the config + login phases.

• This may make the build/login path more consistent with regard to the OCM attributes they use. Rebuilding/refetching OCM attributes may introduce (small) windows for race conditions.

Which Jira/Github issue(s) does this PR fix?

• Related Issue # OSD-28241,
  SREP-164, SREP-183
• Closes #

Special notes for your reviewer

After initial draft review, additional unit tests should be added.

Unit Test Coverage

Waiting for initial draft review, before adding remaining unit tests.
I've manually tested this branch against OSDCTL using both legacy backplane functions, and new OCM sdk connection fed backplane functions. I've connected to clusters using each path, as well as mixing these within a single util. I've also tested basic ocm-backplane login cli connections in the terminal.

Guidelines

• If it's a new sub-command or new function to an existing sub-command, please cover at least 50% of the code
• If it's a bug fix for an existing sub-command, please cover 70% of the code

Test coverage checks

• Added unit tests
• Created jira card to add unit test
• This PR may not need unit tests

Pre-checks (if applicable)

• Ran unit tests locally
• Validated the changes in a cluster
• Included documentation changes with PR

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-robot]

@nephomaniac: This pull request references OSD-28241 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.20.0" version, but no target version was set.

In response to this:

What type of PR is this?

• Bug
• Feature
• Documentation
• Test Coverage
• Clean Up
• Others

What this PR does / Why we need it?

This PR allows an OCM connection to be provided to the config building and login functions. ie adding new '*WithConn()' function variants allowing the caller to provide the OCM connection. This PR attempts to allow utils sharing backplane code to interact with clusters which reside in different OCM envs within the same executable. The most relevant use case is testing/developing within pre-production STAGE and INTEGRATION OCM envs, that are serviced by Hive clusters which exist in OCM Production environment(s)

• Today as part of the client connection setup/code-path, it is not possible to create a clean separation of OCM config to be provided to multiple backplane connections within a single executable. It appears that through a chain of functions, the use of env vars prevents this from working as desired.

• These changes also help avoid rebuilding/tearing down multiple OCM connections during the config + login phases.

• This may make the build/login path more consistent with regard to the OCM attributes they use. Rebuilding/refetching OCM attributes may introduce (small) windows for race conditions.

Which Jira/Github issue(s) does this PR fix?

• Related Issue # OSD-28241,
  SREP-164, SREP-183
• Closes #

Special notes for your reviewer

After initial draft review, additional unit tests should be added.

Unit Test Coverage

Waiting for initial draft review, before adding remaining unit tests.
I've manually tested this branch against OSDCTL using both legacy backplane functions, and new OCM sdk connection fed backplane functions. I've connected to clusters using each path, as well as mixing these within a single util. I've also tested basic ocm-backplane login cli connections in the terminal.

Guidelines

• If it's a new sub-command or new function to an existing sub-command, please cover at least 50% of the code
• If it's a bug fix for an existing sub-command, please cover 70% of the code

Test coverage checks

• Added unit tests
• Created jira card to add unit test
• This PR may not need unit tests

Pre-checks (if applicable)

• Ran unit tests locally
• Validated the changes in a cluster
• Included documentation changes with PR

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.

[typeid]

Not a detailed review and not a blocker for this PR:
given recent discussions, we really need a refactor the logic we share across tools out of /cmd/ into /pkg/ - and changes to these functions with care and knowledge around their consumers (CAD, osdctl).

[feichashao]

Hi @nephomaniac , could you check the comments by @samanthajayasinghe when you have capacity? Thanks!

[samanthajayasinghe]

Let's hold this PR for now

[samanthajayasinghe]

/hold

[nephomaniac]

Hi @samanthajayasinghe sorry for the delay on this. I took some time to investigate a larger refactor, but not sure it's appropriate for this specific change/benefit. Please let me know if there's anything I can do to move this (or similar/smaller change forward). Thank you!