Skip to content

OpenID Credential Configuration #248

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

Closed
stefenAMZ opened this issue Dec 1, 2022 · 6 comments · Fixed by #252
Closed

OpenID Credential Configuration #248

stefenAMZ opened this issue Dec 1, 2022 · 6 comments · Fixed by #252
Labels
enhancement New feature or request

Comments

@stefenAMZ
Copy link

Currently when utilizing OpenID instead of the standard Key_ID/Access_key workflow fails with

Missing credentials in config, if using AWS_CONFIG_FILE, set AWS_SDK_LOAD_CONFIG=1

would be nice to be able to grab credentials from "aws-actions/configure-aws-credentials@v1" if not using id/key in aws configuration.

@gagoar
Copy link
Owner

gagoar commented Dec 1, 2022

Hey @stefenAMZ Lemme see how I can implement that.

@gagoar gagoar added the enhancement New feature or request label Dec 1, 2022
@thenanor
Copy link

thenanor commented Mar 1, 2023

Hey @stefenAMZ and @gagoar, I was able to do this by using the env variables set by the "aws-actions/configure-aws-credentials@v1"

      - name: Assume AWS role
        uses: aws-actions/configure-aws-credentials@v1
        with:
          role-to-assume: the_arn_of_the_role_you_want_to_assume
          aws-region: eu-west-1

      - name: Invoke the Lambda
        uses: gagoar/invoke-aws-lambda@master
        with:
          AWS_ACCESS_KEY_ID: ${{env.AWS_ACCESS_KEY_ID}}
          AWS_SECRET_ACCESS_KEY: ${{env.AWS_SECRET_ACCESS_KEY}}
          AWS_SESSION_TOKEN: ${{env.AWS_SESSION_TOKEN}}
          REGION: ${{env.AWS_REGION}}
          FunctionName: foobarFunction
          Payload: '{ "myParameter": false }'

@gagoar
Copy link
Owner

gagoar commented Mar 1, 2023

Thank you @thenanor! So the action configure-aws-credentials will provide the necessary envs after invoking it? That's very convenient.

We just need to document this in the readme. @stefenAMZ does this will satisfy your request?

@schammah
Copy link

schammah commented Oct 6, 2023

@gagoar the thing is that this definition is kind of duplicate, aws-cli can automatically detect the env variables if they are configured with `"aws-actions/configure-aws-credentials@v3" which is what everyone is opting towards now, short lived session token credentials.

I think in addition to the documentation change, the then there isn't any need to even define AWS_ACCESS_KEY_ID and AWS_SECRET_ACCESS_KEY and they shouldn't be a required variables even.
If someone does need to define them similarly to now they can do it in the env portion versus the with now.
i.e. 2 scenarios:

#1

      - name: Assume AWS role
        uses: aws-actions/configure-aws-credentials@v3
        with:
          role-to-assume: the_arn_of_the_role_you_want_to_assume
          aws-region: us-east-1

      - name: Invoke the Lambda
        uses: gagoar/invoke-aws-lambda@master
        with:
          REGION: ${{env.AWS_REGION}}
          FunctionName: foobarFunction
          Payload: '{ "myParameter": false }'

#2 - people not using configure aws creds

      - name: Invoke the Lambda
        uses: gagoar/invoke-aws-lambda@master
        env:
          AWS_ACCESS_KEY_ID: ${{env.AWS_ACCESS_KEY_ID}}
          AWS_SECRET_ACCESS_KEY: ${{env.AWS_SECRET_ACCESS_KEY}}
        with:
          REGION: ${{env.AWS_REGION}}
          FunctionName: foobarFunction
          Payload: '{ "myParameter": false }'

since its not fully backward compatible, might need to release an updated version of this
also its better in the documentation and examples to let people refer to the action at a specific tag/version and not to master, like in this case it may break some use cases, actually maybe if you just keep the variables definitions but not make them required this may not break

@gagoar
Copy link
Owner

gagoar commented Nov 7, 2023

@gagoar the thing is that this definition is kind of duplicate, aws-cli can automatically detect the env variables if they are configured with `"aws-actions/configure-aws-credentials@v3" which is what everyone is opting towards now, short lived session token credentials.

I think in addition to the documentation change, the then there isn't any need to even define AWS_ACCESS_KEY_ID and AWS_SECRET_ACCESS_KEY and they shouldn't be a required variables even. If someone does need to define them similarly to now they can do it in the env portion versus the with now. i.e. 2 scenarios:

#1

      - name: Assume AWS role
        uses: aws-actions/configure-aws-credentials@v3
        with:
          role-to-assume: the_arn_of_the_role_you_want_to_assume
          aws-region: us-east-1

      - name: Invoke the Lambda
        uses: gagoar/invoke-aws-lambda@master
        with:
          REGION: ${{env.AWS_REGION}}
          FunctionName: foobarFunction
          Payload: '{ "myParameter": false }'

#2 - people not using configure aws creds

      - name: Invoke the Lambda
        uses: gagoar/invoke-aws-lambda@master
        env:
          AWS_ACCESS_KEY_ID: ${{env.AWS_ACCESS_KEY_ID}}
          AWS_SECRET_ACCESS_KEY: ${{env.AWS_SECRET_ACCESS_KEY}}
        with:
          REGION: ${{env.AWS_REGION}}
          FunctionName: foobarFunction
          Payload: '{ "myParameter": false }'

since its not fully backward compatible, might need to release an updated version of this also its better in the documentation and examples to let people refer to the action at a specific tag/version and not to master, like in this case it may break some use cases, actually maybe if you just keep the variables definitions but not make them required this may not break

I understand. What if we add a new parameter so the passed credentials, as we usually do, aren't needed? Would that sort out your use case?

I'm thinking an option like OPEN_ID_ENABLED=boolean

@gagoar
Copy link
Owner

gagoar commented Nov 7, 2023

@gagoar The thing is that this definition is kind of duplicate, aws-cli can automatically detect the env variables if they are configured with "aws-actions/configure-aws-credentials@v3" which is what everyone is opting towards now, short lived session token credentials. I think in addition to the documentation change, the then there isn't any need to even define AWS_ACCESS_KEY_IDandAWS_SECRET_ACCESS_KEYand they shouldn't be arequiredvariables even. If someone does need to define them similarly to now they can do it in theenvportion versus thewith` now. i.e. 2 scenarios:
#1

      - name: Assume AWS role
        uses: aws-actions/configure-aws-credentials@v3
        with:
          role-to-assume: the_arn_of_the_role_you_want_to_assume
          aws-region: us-east-1

      - name: Invoke the Lambda
        uses: gagoar/invoke-aws-lambda@master
        with:
          REGION: ${{env.AWS_REGION}}
          FunctionName: foobarFunction
          Payload: '{ "myParameter": false }'

#2 - people not using configure aws creds

      - name: Invoke the Lambda
        uses: gagoar/invoke-aws-lambda@master
        env:
          AWS_ACCESS_KEY_ID: ${{env.AWS_ACCESS_KEY_ID}}
          AWS_SECRET_ACCESS_KEY: ${{env.AWS_SECRET_ACCESS_KEY}}
        with:
          REGION: ${{env.AWS_REGION}}
          FunctionName: foobarFunction
          Payload: '{ "myParameter": false }'

since its not fully backward compatible, might need to release an updated version of this also its better in the documentation and examples to let people refer to the action at a specific tag/version and not to master, like in this case it may break some use cases, actually maybe if you just keep the variables definitions but not make them required this may not break

I understand. What if we add a new parameter so the passed credentials, as we usually do, aren't needed? Would that sort out your use case?

I'm thinking an option like OPEN_ID_ENABLED=boolean

I don't want to break functionality with the older implementations. There are lots of repositories using @master straight up at the moment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants