OCPBUGS-7860: azure: session: fix unclear auth error messages #6901

r4f4 · 2023-02-24T10:41:32Z

If you tried to authenticate with a certificate and provided the wrong password, the installer error message didn't actually say what the error was. It just said

$ ./openshift-install create install-config --dir ipi-test
INFO Could not get an azure authorizer from file: auth file missing client and certificate credentials
INFO Asking user to provide authentication info
? azure subscription id [? for help]

That happens because the autorest lib would just ignore the auth errors in favor of a generic message [1]. Since the autorest/auth module is going out of support by March 31, 2023, let's get rid of it and load the auth file ourselves.

Now when doing cert authentication with the wrong password we get:

INFO Credentials loaded from file "/root/.azure/osServicePrincipal.json"
WARNING Using client certs to authenticate. Please be warned cluster does not support certs and only the installer does.
ERROR failed to fetch Kubeconfig Admin Client: failed to load asset "Install Config": failed to create install config: creating Azure session: failed to parse client certificate: pkcs12: decryption password incorrect

[1] https://github.com/Azure/go-autorest/blob/main/autorest/azure/auth/auth.go#L264-L270

openshift-ci-robot · 2023-02-24T10:41:37Z

@r4f4: This pull request references Jira Issue OCPBUGS-7860, which is invalid:

expected the bug to target the "4.13.0" version, but no target version was set

Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.

The bug has been updated to refer to the pull request using the external bug tracker.

Details

In response to this:

If you tried to authenticate with a certificate and provided the wrong password, the installer error message didn't actually say what the error was. It just said
$ ./openshift-install create install-config --dir ipi-test
INFO Could not get an azure authorizer from file: auth file missing client and certificate credentials
INFO Asking user to provide authentication info
? azure subscription id [? for help]
That happens because the autorest lib would just ignore the auth errors in favor of a generic message [1]. Since the autorest/auth module is going out of support by March 31, 2023, let's get rid of it and load the auth file ourselves.

Now when doing cert authentication with the wrong password we get:
INFO Credentials loaded from file "/root/.azure/osServicePrincipal.json"
WARNING Using client certs to authenticate. Please be warned cluster does not support certs and only the installer does.
ERROR failed to fetch Kubeconfig Admin Client: failed to load asset "Install Config": failed to create install config: creating Azure session: failed to parse client certificate: pkcs12: decryption password incorrect
[1] https://github.com/Azure/go-autorest/blob/main/autorest/azure/auth/auth.go#L264-L270

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.

r4f4 · 2023-02-24T10:42:42Z

/jira refresh

openshift-ci-robot · 2023-02-24T10:42:50Z

@r4f4: This pull request references Jira Issue OCPBUGS-7860, which is valid. The bug has been moved to the POST state.

3 validation(s) were run on this bug

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

Requesting review from QA contact:
/cc @jinyunma

Details

In response to this:

/jira refresh

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.

r4f4 · 2023-02-24T12:13:23Z

/hold
Ops, forgot to commit the vendor changes. Will do so once the Azure jobs finish.

r4f4 · 2023-02-24T14:11:09Z

/hold cancel

barbacbd · 2023-02-27T15:59:52Z

pkg/asset/installconfig/azure/session.go

nit: Might be able to clean this warning up a bit. "Certs are only supported by the installer, not the cluster". Maybe something like that.

I didn't add this warning, just moved it. It was added in #6250. But I can reword if we think it's necessary.

pkg/asset/installconfig/azure/session.go

The `autorest/auth` library expects specific keys in the json file for certificate authentication [1]. If we ever saved the Credentials object to a json file during survey, it would have used the wrong keys: `certificatePath` and `certificatePassword` instead of `clientCertificate` and `clientCertificatePassword`, respectively. This change aims to fix that so that the saved `osServicePrincipal.json` file with certificate authentication details can be correctly loaded by the autorest library. It'll also be important in a follow-up change where we are going to load the file ourselves instead of using autorest. Notice that this is not breaking compatibility with previous versions since those fields were never used to create a file (we only survey client secret credentials). [1] https://github.com/Azure/go-autorest/blob/main/autorest/azure/auth/auth.go#L348-L349

If you tried to authenticate with a certificate and provided the wrong password, the installer error message didn't actually say what the error was. It just said ``` $ ./openshift-install create install-config --dir ipi-test INFO Could not get an azure authorizer from file: auth file missing client and certificate credentials INFO Asking user to provide authentication info ? azure subscription id [? for help] ``` That happens because the autorest lib would just ignore the auth errors in favor of a generic message [1]. Since the `autorest/auth` module is going out of support by March 31, 2023, let's get rid of it and load the auth file ourselves. Now when doing cert authentication with the wrong password we get: ``` INFO Credentials loaded from file "/root/.azure/osServicePrincipal.json" WARNING Using client certs to authenticate. Please be warned cluster does not support certs and only the installer does. ERROR failed to fetch Kubeconfig Admin Client: failed to load asset "Install Config": failed to create install config: creating Azure session: failed to parse client certificate: pkcs12: decryption password incorrect ``` [1] https://github.com/Azure/go-autorest/blob/main/autorest/azure/auth/auth.go#L264-L270

r4f4 · 2023-02-27T19:26:40Z

Added a new commit to isoloate and explain why the json annotations were changed. No code changes were introduced.

openshift-ci · 2023-02-27T23:54:57Z

@r4f4: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/okd-scos-e2e-aws-upgrade	`fcaccfa`	link	false	`/test okd-scos-e2e-aws-upgrade`
ci/prow/okd-scos-e2e-aws-ovn	`fcaccfa`	link	false	`/test okd-scos-e2e-aws-ovn`
ci/prow/okd-e2e-aws-ovn-upgrade	`fcaccfa`	link	false	`/test okd-e2e-aws-ovn-upgrade`
ci/prow/e2e-azurestack	`fcaccfa`	link	false	`/test e2e-azurestack`

Full PR test history. Your PR dashboard.

Details

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.

patrickdillon · 2023-02-28T01:53:15Z

/approve

openshift-ci · 2023-02-28T01:54:44Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: patrickdillon

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [patrickdillon]

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

patrickdillon · 2023-02-28T14:18:10Z

/lgtm
e2e-azure-ovn was successful install. just one e2e test failure re registry.

openshift-ci-robot · 2023-02-28T15:00:39Z

/retest-required

Remaining retests: 0 against base HEAD 05a6325 and 2 for PR HEAD fcaccfa in total

openshift-ci-robot · 2023-03-01T01:44:41Z

/retest-required

Remaining retests: 0 against base HEAD 1445b3b and 1 for PR HEAD fcaccfa in total

openshift-ci-robot · 2023-03-01T09:04:21Z

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

openshift/installer#6901

Jira Issue OCPBUGS-7860 has been moved to the MODIFIED state.

Details

In response to this:

If you tried to authenticate with a certificate and provided the wrong password, the installer error message didn't actually say what the error was. It just said
$ ./openshift-install create install-config --dir ipi-test
INFO Could not get an azure authorizer from file: auth file missing client and certificate credentials
INFO Asking user to provide authentication info
? azure subscription id [? for help]
That happens because the autorest lib would just ignore the auth errors in favor of a generic message [1]. Since the autorest/auth module is going out of support by March 31, 2023, let's get rid of it and load the auth file ourselves.

Now when doing cert authentication with the wrong password we get:
INFO Credentials loaded from file "/root/.azure/osServicePrincipal.json"
WARNING Using client certs to authenticate. Please be warned cluster does not support certs and only the installer does.
ERROR failed to fetch Kubeconfig Admin Client: failed to load asset "Install Config": failed to create install config: creating Azure session: failed to parse client certificate: pkcs12: decryption password incorrect
[1] https://github.com/Azure/go-autorest/blob/main/autorest/azure/auth/auth.go#L264-L270

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 bot requested review from jhixson74 and mtulio February 24, 2023 10:42

openshift-ci bot requested a review from jinyunma February 24, 2023 10:42

openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 24, 2023

openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 24, 2023

barbacbd reviewed Feb 27, 2023

View reviewed changes

rna-afk reviewed Feb 27, 2023

View reviewed changes

pkg/asset/installconfig/azure/session.go Outdated Show resolved Hide resolved

r4f4 added 3 commits February 27, 2023 20:24

azure: session: vendor changes

fcaccfa

r4f4 force-pushed the azure-remove-autorest-auth branch from 6abfb63 to fcaccfa Compare February 27, 2023 19:24

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 28, 2023

openshift-ci bot assigned patrickdillon Feb 28, 2023

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Feb 28, 2023

openshift-merge-robot merged commit 9a5ef71 into openshift:master Mar 1, 2023

OCPBUGS-7860: azure: session: fix unclear auth error messages #6901

OCPBUGS-7860: azure: session: fix unclear auth error messages #6901

Uh oh!

Conversation

r4f4 commented Feb 24, 2023

Uh oh!

openshift-ci-robot commented Feb 24, 2023

Uh oh!

r4f4 commented Feb 24, 2023

Uh oh!

openshift-ci-robot commented Feb 24, 2023

Uh oh!

r4f4 commented Feb 24, 2023

Uh oh!

r4f4 commented Feb 24, 2023

Uh oh!

barbacbd Feb 27, 2023

Choose a reason for hiding this comment

Uh oh!

r4f4 Feb 27, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

r4f4 commented Feb 27, 2023

Uh oh!

openshift-ci bot commented Feb 27, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

patrickdillon commented Feb 28, 2023

Uh oh!

openshift-ci bot commented Feb 28, 2023

Uh oh!

patrickdillon commented Feb 28, 2023

Uh oh!

openshift-ci-robot commented Feb 28, 2023

Uh oh!

openshift-ci-robot commented Mar 1, 2023

Uh oh!

openshift-ci-robot commented Mar 1, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

openshift-ci bot commented Feb 27, 2023 •

edited

Loading