Skip to content
This repository was archived by the owner on Oct 24, 2023. It is now read-only.

fix(auth): Explicitly import the AAD library to allow cluster upgrades#1474

Merged
mboersma merged 1 commit intoAzure:masterfrom
skinny:aad-upgrade-support
Jun 14, 2019
Merged

fix(auth): Explicitly import the AAD library to allow cluster upgrades#1474
mboersma merged 1 commit intoAzure:masterfrom
skinny:aad-upgrade-support

Conversation

@skinny
Copy link
Contributor

@skinny skinny commented Jun 13, 2019

After #506 was closed/merged it's still not possible to upgrade clusters with AAD support enabled. During the upgrade or scaling operation the error

FATA[0377] Error upgrading cluster: No Auth Provider found for name "azure"

still appears. This PR fixes that.

@acs-bot acs-bot added the size/L label Jun 13, 2019
@mboersma
Copy link
Member

It looks like some symlinks got checked in as files in this PR, which will cause problems when checking out to UNIX-y platforms. I'm not entirely sure how to fix that, but maybe it's related to the core.symlinks git setting?

@skinny
Copy link
Contributor Author

skinny commented Jun 13, 2019

Yeah I didnt know what to do with those either. They got changed when I updated the deps and build the project so I reckoned they should be committed as well

Copy link
Contributor

@CecileRobertMichon CecileRobertMichon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/azp run pr-e2e

@CecileRobertMichon
Copy link
Contributor

/azp run pr-e2e

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@codecov
Copy link

codecov bot commented Jun 13, 2019

Codecov Report

Merging #1474 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1474   +/-   ##
=======================================
  Coverage   75.58%   75.58%           
=======================================
  Files         128      128           
  Lines       18136    18136           
=======================================
  Hits        13709    13709           
  Misses       3636     3636           
  Partials      791      791

@mboersma
Copy link
Member

mboersma commented Jun 14, 2019

Here's the problem I see:

$ git co -b aad-upgrade-support skinny/aad-upgrade-support
error: unable to create symlink vendor/github.com/go-bindata/go-bindata/testdata/symlinkFile/file1: File name too long
error: unable to create symlink vendor/github.com/go-bindata/go-bindata/testdata/symlinkParent/symlinkTarget: File name too long
error: unable to create symlink vendor/github.com/go-bindata/go-bindata/testdata/symlinkRecursiveParent/symlinkTarget: File name too long
D	vendor/github.com/go-bindata/go-bindata/testdata/symlinkFile/file1
D	vendor/github.com/go-bindata/go-bindata/testdata/symlinkParent/symlinkTarget
D	vendor/github.com/go-bindata/go-bindata/testdata/symlinkRecursiveParent/symlinkTarget
Branch 'aad-upgrade-support' set up to track remote branch 'aad-upgrade-support' from 'skinny'.
Switched to a new branch 'aad-upgrade-support'

Maybe not a critical issue since the links are in a vendored testdata folder, but IDK if this will cause future merge issues. I'll try to re-vendor on top of this from macOS and update the PR if I can.

Update: replaying this on macOS worked fine, but I couldn't add the commit on top of the broken commit, so I replaced it and force-pushed. Hope that's cool @skinny.

@mboersma mboersma force-pushed the aad-upgrade-support branch from 90ccfda to 9469fbe Compare June 14, 2019 18:12
@CecileRobertMichon
Copy link
Contributor

/azp run pr-e2e

@CecileRobertMichon
Copy link
Contributor

/lgtm

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@acs-bot
Copy link

acs-bot commented Jun 14, 2019

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: CecileRobertMichon, skinny

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:

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

@skinny
Copy link
Contributor Author

skinny commented Jun 14, 2019

@mboersma no problem! As long as the end result is the same its fine by me 😊

Thanks and have a great weekend

@mboersma
Copy link
Member

Codecov didn't report back on this PR, so the bot won't merge it. I'll do that.

@mboersma mboersma merged commit a037bb7 into Azure:master Jun 14, 2019
@skinny skinny deleted the aad-upgrade-support branch June 14, 2019 22:20
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants