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

1.6.0 breaks when azure/client ID not known at workflow start #404

Closed
mhulscher opened this issue Jan 16, 2024 · 4 comments
Closed

1.6.0 breaks when azure/client ID not known at workflow start #404

mhulscher opened this issue Jan 16, 2024 · 4 comments
Assignees
Labels
question The issue doesn't require a change to the product in order to be resolved. Most issues start as that

Comments

@mhulscher
Copy link

mhulscher commented Jan 16, 2024

After (automatically) upgrading to 1.6.0 our workflows started breaking today.

We have a job that first reads environment data from a JSON file, which we later use in our azure/login action. This no longer works with 1.6.0 because it uses a pre: step.

      - id: settings
        name: Read settings
        uses: ActionsTools/read-json-action@main
        with:
          file_path: settings.json

      - name: Log in to Azure using OIDC
        uses: azure/login@v1
        with:
          tenant-id: ${{ fromJSON(steps.settings.outputs.environments).dev.tenant_id }}
          client-id: ${{ fromJSON(steps.settings.outputs.environments).dev.ci_spn_client_id }}
          allow-no-subscriptions: true

As a temporary workaround we now pin the action to the latest 1.5.

In my opinion this change could have warranted a major version bump.

@mhulscher mhulscher added the need-to-triage Requires investigation label Jan 16, 2024
@kristeey
Copy link

related to: #403

@YanaXu
Copy link
Collaborator

YanaXu commented Jan 17, 2024

It's another issue related to v1.6.0. Let's fix it in v1.6.1.

@YanaXu YanaXu closed this as completed Jan 17, 2024
@YanaXu YanaXu reopened this Jan 17, 2024
@YanaXu YanaXu added bug Something isn't working and removed need-to-triage Requires investigation labels Jan 17, 2024
@MoChilia
Copy link
Member

Hi @mhulscher,
Before using an evaluation expression, kindly verify the existence of its input. In your case, we should first check if steps.settings.outputs.environments exists, like

tenant-id: ${{ steps.settings.outputs.environments && fromJSON(steps.settings.outputs.environments).dev.tenant_id }}

This precaution can help prevent potential issues in case the previous step fails. See actions/runner#1722 (comment).

@MoChilia MoChilia added question The issue doesn't require a change to the product in order to be resolved. Most issues start as that and removed bug Something isn't working labels Jan 17, 2024
@mhulscher
Copy link
Author

Hi @MoChilia , I expect azure/login to fail the job if one of its inputs is nil, that's totally fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question The issue doesn't require a change to the product in order to be resolved. Most issues start as that
Projects
None yet
Development

No branches or pull requests

4 participants