Skip to content
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

config: drop support for deprecated Azure's service principal authentication #1906

Merged
merged 13 commits into from
Jun 14, 2023

Conversation

elchead
Copy link
Contributor

@elchead elchead commented Jun 9, 2023

Context

This is the last step to completely deprecate the old method of handling Azure service accounts (see more on the migration here). With the next release the CLI enforces the user to update its configuration to the new unified UAMI.

Proposed change(s)

  • err when setting app registration fields config file and warn when setting ENV variable
  • update docs

Additional info

AB#2967

Checklist

  • Update docs
  • Add labels (e.g., for changelog category)
  • Link to Milestone

@elchead elchead requested a review from derpsteb as a code owner June 9, 2023 15:06
@netlify
Copy link

netlify bot commented Jun 9, 2023

Deploy Preview for constellation-docs canceled.

Name Link
🔨 Latest commit 4366399
🔍 Latest deploy log https://app.netlify.com/sites/constellation-docs/deploys/6489d8e75b0394000974ee9a

@elchead elchead requested a review from katexochen as a code owner June 12, 2023 06:39
@elchead elchead added the breaking change Change breaks existing API or configuration. label Jun 12, 2023
@elchead elchead added this to the v2.9.0 milestone Jun 12, 2023
@elchead elchead force-pushed the config/azure/remove-app-client-id branch from 4b99dab to 04d20cf Compare June 12, 2023 06:56
@katexochen katexochen removed their request for review June 12, 2023 07:00
@elchead elchead requested a review from 3u13r June 12, 2023 07:52
internal/config/config.go Outdated Show resolved Hide resolved
@elchead elchead changed the title config: invalidate app client id field for azure and provide error info config: invalidate app registration fields for azure and provide error info Jun 13, 2023
@elchead elchead requested a review from derpsteb June 13, 2023 07:59
internal/config/config.go Outdated Show resolved Hide resolved
@derpsteb
Copy link
Member

Looks a lot cleaner now. I like it! 👍

@elchead elchead marked this pull request as draft June 13, 2023 08:26
@elchead elchead marked this pull request as ready for review June 13, 2023 09:30
@elchead elchead requested a review from thomasten as a code owner June 13, 2023 09:30
@elchead
Copy link
Contributor Author

elchead commented Jun 13, 2023

linker fails due to timeout of external HTTP link. can ignore

@elchead elchead changed the title config: invalidate app registration fields for azure and provide error info config: invalidate app registration fields for azure and update docs Jun 13, 2023
internal/config/config.go Outdated Show resolved Hide resolved
Copy link
Member

@derpsteb derpsteb left a comment

Choose a reason for hiding this comment

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

looks good from my side, maybe @3u13r wants to take a quick look. he is more familiar with the topic.

Please update the PR description to give more context. This should be understandable by a user who clicked the PR link while reading the changelog.

Copy link
Contributor

@malt3 malt3 left a comment

Choose a reason for hiding this comment

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

LGTM

@derpsteb
Copy link
Member

Please add some more information to the PR description for users who are redirected from the changelog. They can't see the linked ticket. We probably describe the topic in the docs somewhere.

@elchead elchead changed the title config: invalidate app registration fields for azure and update docs config: drop support for deprecated Azure's service principal authentication Jun 14, 2023
@elchead elchead merged commit 07de648 into main Jun 14, 2023
@elchead elchead deleted the config/azure/remove-app-client-id branch June 14, 2023 15:50
msanft pushed a commit that referenced this pull request Jun 19, 2023
…ication (#1906)

* invalidate app client id field for azure and provide info

* remove TestNewWithDefaultOptions case

* fix test

* remove appClientID field

* remove client secret + rename err

* remove from docs

* otto feedback

* update docs

* delete env test in cfg since no envs set anymore

* Update dev-docs/workflows/github-actions.md

Co-authored-by: Otto Bittner <[email protected]>

* WARNING to stderr

* fix check

---------

Co-authored-by: Otto Bittner <[email protected]>
msanft pushed a commit that referenced this pull request Jun 19, 2023
…ication (#1906)

* invalidate app client id field for azure and provide info

* remove TestNewWithDefaultOptions case

* fix test

* remove appClientID field

* remove client secret + rename err

* remove from docs

* otto feedback

* update docs

* delete env test in cfg since no envs set anymore

* Update dev-docs/workflows/github-actions.md

Co-authored-by: Otto Bittner <[email protected]>

* WARNING to stderr

* fix check

---------

Co-authored-by: Otto Bittner <[email protected]>
msanft pushed a commit that referenced this pull request Jun 20, 2023
…ication (#1906)

* invalidate app client id field for azure and provide info

* remove TestNewWithDefaultOptions case

* fix test

* remove appClientID field

* remove client secret + rename err

* remove from docs

* otto feedback

* update docs

* delete env test in cfg since no envs set anymore

* Update dev-docs/workflows/github-actions.md

Co-authored-by: Otto Bittner <[email protected]>

* WARNING to stderr

* fix check

---------

Co-authored-by: Otto Bittner <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change Change breaks existing API or configuration.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants