feature(azure devops): support multiple organisations

[sanderaernouts]

Hey, I just made a Pull Request!

I Added an AzureDevOpsCredentialProvider, similar to the GitHub and AWS integrations, that can return a token based on the provided URL. This PR resolves #10431 by adding support for organisations to the Azure integration config.

The updated Azure integration configuration looks like this:

integrations:
  azure:
  - host: my.devops.server
    credentials:
    - token: my-pat
  - host: dev.azure.com
    credentials:
    - clientId: <id>
      clientSecret: <secret>
      tenantId: <tenant>
      organisations:
      - my-org
      - my-other-org
      - yet-another-org
    - token: my-org-pat
      organisations:
      - my-other-org

The current token and credential fields in the Azure integration config are deprecated in favour of the credentials field

✔️ Checklist

• A changeset describing the change and affected packages. (more info)
• Added or updated documentation
• Tests for new functionality and regression tests for bug fixes
• Screenshots attached (for UI changes)
• All your commits have a Signed-off-by line in the message. (more info)

[backstage-goalie]

Changed Packages

Package Name	Package Path	Changeset Bump	Current Version
@backstage/backend-common	packages/backend-common	patch	v0.19.2
@backstage/integration	packages/integration	minor	v1.6.0
@backstage/plugin-catalog-backend-module-azure	plugins/catalog-backend-module-azure	patch	v0.1.19
@backstage/plugin-scaffolder-backend	plugins/scaffolder-backend	patch	v1.16.0

[backstage-goalie]

Thanks for the contribution!
All commits need to be DCO signed before they are reviewed. Please refer to the the DCO section in CONTRIBUTING.md or the DCO status for more info.

[awanlin]

Thanks for taking this on @sanderaernouts, I do have a question for you about these changes.

In the PR description you have:

integrations:
  azure:
    host: dev.azure.com
    credentials: 
    - organisations:
      - my-org
      token: my-pat 
    - organisations:
      - my-other-org
      token: my-other-pat

Are you saying that you would be removing the ability to have an array of host? If so that's not ideal as it would make it harder to be able to use Azure DevOps Services and Server instances together with Backstage. This is the exact scenario we have right now and is not an uncommon setup

[sanderaernouts]

@awanlin, no, I won't be touching that part. I typed the YAML config example from memory and forgot that the Azure integration config is an array of configs. To clarify, I plan to replace the token and credential fields with a single credentials field. This field is an array of token, service principal or managed identity credentials. The credentials will have an optional organisations field where you can specify which Azure DevOps organisation the credential applies.

integrations:
  azure:
    - host: dev.azure.com
      credentials: 
      - organisations:
        - my-org
        token: my-pat 
      - organisations:
        - my-other-org
        token: my-other-pat
    - host: some.devops.server
      credentials: 
      - token: my-pat 

I have updated the PR description 👍

[awanlin]

Awesome, thanks for the follow up @sanderaernouts. Another question popup though: why the extra organizations?

Could this:

integrations:
  azure:
    - host: dev.azure.com
      credentials: 
      - organisations:
        - my-org
        token: my-pat 
      - organisations:
        - my-other-org
        token: my-other-pat
    - host: some.devops.server
      credentials: 
      - token: my-pat 

Be something like this instead:

integrations:
  azure:
    - host: dev.azure.com
      credentials: 
        - organization: my-org
          token: my-pat 
        - organization: my-other-org
          token: my-other-pat
    - host: some.devops.server
      credentials:
        - organization: my-org
          token: my-pat

With Azure DevOps Server the concept of Team Collections is more or less the same as Organizations in Azure DevOps Services. I think going this way seeing as we will have a breaking change makes this more consistent

[sanderaernouts]

@awanlin I added an array of organizations to support reusing the same service principal or managed identity for multiple organizations. A single service principal or managed identity could be used as long as the same Azure AD tenant backs the Azure DevOps organizations.

However, I'm also okay with organization; you can still duplicate the service principal or managed identity.

[awanlin]

You're last comment helped me better understand what you where trying to do with the config, that works for me then 👍

Maybe we update the description with this then:

integrations:
  azure:
    - host: dev.azure.com
      credentials: 
      - organisations:
        - my-org
        - my-org-related
        - my-org-another-related
        token: my-pat 
      - organisations:
        - my-other-org
        token: my-other-pat
    - host: some.devops.server
      credentials: 
      - token: my-pat 

That shows the credential reuse a little better (well it does for me 😉 )

[sanderaernouts]

@awanlin I updated the PR description and will use similar examples when updating the docs. I have also added an example for the Azure DevOps server. Meanwhile, I made good progress on the credential provider yesterday, so there already is some stuff to review if you want to.

[Rugvip]

Nice, great to get this added for azure too! 👍

Couple of initial comments. In particular I think it's best if we split out renames of existing systems in a new PR to avoid breaking changes, because I don't think this overall is something that we can ship straight into a main-line release.

[github-actions]

Uffizzi Preview deployment-33710 was deleted.

[sanderaernouts]

@awanlin, I updated the docs as well. Should we restrict personal access token (PAT) credentials to zero or one organization in the integration config? For Azure DevOps, the PAT is organization-specific, so filling out more than one organization makes no sense.

[Rugvip]

Nice, thank you! 🎉

Just two nits. It would be nice to have some clarity on #18213 (comment) but I don't feel it's a requirement

There are a lot of PRs we want to get into 1.17 😅

[sanderaernouts]

Nice, thank you! 🎉

Just two nits. It would be nice to have some clarity on #18213 (comment) but I don't feel it's a requirement

There are a lot of PRs we want to get into 1.17 😅

@Rugvip, thanks; I know you guys are busy. I'd prefer to have #18213 (comment) fixed resolved before merging.

I can imagine many contributors are pushing for 1.17, but my PR is the most important one to get merged 😜.

[Rugvip]

✨

[awanlin]

I'd just like to note my worries about this getting into the release today. I strongly feel that this needs to be included in at least one of the weekly releases.

[Rugvip]

@awanlin will do!

[awanlin]

@Rugvip, this is ready to be merged. Also, it would be good to include this in the release notes for 1.18.0, how do we make sure that happens?

[Rugvip]

@awanlin nice! 🎉

It'll be in the release notes, I've added a section to our internal draft of the notes

[github-actions]

Thank you for contributing to Backstage! The changes in this pull request will be part of the 1.18.0 release, scheduled for Tue, 19 Sep 2023.

[awanlin]

Thanks @Rugvip, and many, many, many thanks to @sanderaernouts for the work done in this PR! 🚀

[sanderaernouts]

Thanks, @awanlin and @Rugvip, for your time and feedback 🚀 🎉