feat: make proxy configurable #267
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #267 +/- ##
==========================================
- Coverage 48.53% 47.82% -0.72%
==========================================
Files 52 52
Lines 3441 3467 +26
==========================================
- Hits 1670 1658 -12
- Misses 1510 1548 +38
Partials 261 261
|
typeid
changed the title
openshift-ci
bot
added
the
do-not-merge/work-in-progress
typeid
changed the title
openshift-ci
bot
removed
the
do-not-merge/work-in-progress
|
Removed WIP label. Tested that this works programatically and tested that I can still create console/credentials for clusters. |
typeid
force-pushed
the
configurable_proxy
branch
from
088c6c2
to
a2ee797
Compare
|
/label tide/merge-method-squash |
openshift-ci
bot
added
the
tide/merge-method-squash
| if err != nil { | ||
| return aws.Credentials{}, fmt.Errorf("failed to assume role sequence: %w", err) | ||
| } | ||
| return targetCredentials, nil | ||
| } | ||
|
|
||
| func isIsolatedBackplaneAccess(cluster *cmv1.Cluster) (bool, error) { | ||
| func isIsolatedBackplaneAccess(cluster *cmv1.Cluster, ocmConnection *ocmsdk.Connection) (bool, error) { |
There was a problem hiding this comment.
| func isIsolatedBackplaneAccess(cluster *cmv1.Cluster, ocmConnection *ocmsdk.Connection) (bool, error) { | |
| func (cfg *QueryConfig) isIsolatedBackplaneAccess(cluster *cmv1.Cluster) (bool, error) { |
Any reason to not make this (and kind of all the other functions) methods off of QueryConfig? To me, that's one of the main benefits of defining a QueryConfig in the first place - you don't have to worry about passing the right arguments?
I could be convinced otherwise though, currently I'm thinking as long as the public/external helper functions are methods that's good enough for me (e.g. skip this one and refactor GetAWSV2Config to be a method).
There was a problem hiding this comment.
Agree that it makes sense, I was being a bit too careful with refactoring I suppose.
The following are now members of QueryConfig (and have been moved to common.go:
getCloudCredentials-> publicgetCloudCredentialsFromBackplaneAPIgetCloudConsole-> public
I also refactored GetCloudConsole the same way GetCloudCredentials has been refactored to enable calling it from console.go, and moved some logic into the new getCloudConsoleFromBackplaneAPI to make it less confusing.
After these changes I tested the following:
Old flow:
- ocm backplane cloud credentials
- ocm backplane cloud console
Non STS clusters:
- ocm backplane cloud credentials
- ocm backplane cloud console
New flow:
- ocm backplane cloud credentials
- ocm backplane cloud console
|
@typeid: 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/test-infra repository. I understand the commands that are listed here. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mjlshen, typeid 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:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
the
approved
What type of PR is this?
cleanup
What this PR does / Why we need it?
We need the proxy to be configurable for programmatic access. Beyond that, this PR also allows passing the whole necessary configuration to create an AWSV2Config, instead of picking up config chunks through ENVs and external config files.
Needed for https://issues.redhat.com/browse/OSD-19388\
Which Jira/Github issue(s) does this PR fix?
Resolves #
Special notes for your reviewer
Pre-checks (if applicable)
After these changes I tested the following:
Old flow:
Non STS clusters:
New flow: