-
Notifications
You must be signed in to change notification settings - Fork 192
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
AGENT-869: Implement a new auth type for ABI #6174
base: master
Are you sure you want to change the base?
AGENT-869: Implement a new auth type for ABI #6174
Conversation
@pawanpinjarkar: This pull request references AGENT-869 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 story to target the "4.16.0" version, but no target version was set. In response to this:
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. |
afb8392
to
34ad1f1
Compare
e206f44
to
3940b8e
Compare
/hold |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #6174 +/- ##
==========================================
- Coverage 68.27% 68.26% -0.01%
==========================================
Files 241 242 +1
Lines 35873 35925 +52
==========================================
+ Hits 24493 24525 +32
- Misses 9215 9232 +17
- Partials 2165 2168 +3
|
3940b8e
to
3ba8f9d
Compare
/hold cancel |
@pawanpinjarkar: This pull request references AGENT-869 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 story to target the "4.16.0" version, but no target version was set. In response to this:
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. |
@pawanpinjarkar: This pull request references AGENT-869 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 story to target the "4.16.0" version, but no target version was set. In response to this:
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. |
/hold cancel |
/cc @carbonin |
a586922
to
a69d6a9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add tests for the new authenticator.
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: pawanpinjarkar The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
- Introduced a new auth type 'agent-installer-local' for agent based installer - This new auth type is mostly similar to the local auth type - agent based installer generates the necessary keys and JWT tokens and pass it to assisted service.
773090e
to
188119a
Compare
@pawanpinjarkar: This pull request references AGENT-869 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 story to target the "4.16.0" version, but no target version was set. In response to this:
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. |
@pawanpinjarkar: This pull request references AGENT-869 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 story to target the "4.16.0" version, but no target version was set. In response to this:
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. |
2a0485c
to
c8be83f
Compare
c8be83f
to
ff6deee
Compare
/retest-required |
@pawanpinjarkar: This pull request references AGENT-869 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 story to target the "4.16.0" version, but no target version was set. In response to this:
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. |
@pawanpinjarkar: The following tests failed, say
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. |
@@ -756,7 +756,7 @@ func (b *bareMetalInventory) GetInfraEnvDownloadURL(ctx context.Context, params | |||
|
|||
func (b *bareMetalInventory) generateShortImageDownloadURL(infraEnvID, imageType, version, arch, imageTokenKey string) (string, *strfmt.DateTime, error) { | |||
switch b.authHandler.AuthType() { | |||
case auth.TypeLocal: | |||
case auth.TypeLocal, auth.TypeAgentLocal: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you ever using the download URL we generate?
@@ -30,7 +30,7 @@ func AgentToken(resource interface{}, authType auth.AuthType) (token string, err | |||
switch authType { | |||
case auth.TypeRHSSO: | |||
token, err = cloudPullSecretToken(pullSecret) | |||
case auth.TypeLocal: | |||
case auth.TypeLocal, auth.TypeAgentLocal: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is also only ever used when building the discovery ISO. Are you guys using that?
log.WithError(err).Fatal("Error decoding private key:") | ||
} | ||
cfg.ECPrivateKeyPEM = string(decodedECPrivateKeyPEM) | ||
os.Setenv("EC_PRIVATE_KEY_PEM", string(decodedECPrivateKeyPEM)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I really don't think this is a good idea.
It looks like you may not actually need the private key at all since you're creating all of the tokens you need from outside the application.
Maybe just don't bother with the private key?
pass it to assisted service.
List all the issues related to this PR
What environments does this code impact?
How was this code tested?
The changes are tested with other relevant changes from agent based installer. The cluster was successfully installed with authentication setup end to end.
Checklist
docs
, README, etc)Reviewers Checklist