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

Return secrets as outputs instead of values #36

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

Olfi01
Copy link

@Olfi01 Olfi01 commented May 12, 2023

Issue #14: Environment variables vs outputs

Description of changes:
Secrets are now returned as action outputs. This results in the following improvements:

  • User side:
    • Less concern needed over which steps to separate in a different job to avoid access to environment variables
    • Full control over where the returned secrets are used
  • Codebase side:
    • No more need to clean up environment variable secrets, as the GitHub Actions toolkit will handle the outputs.

README.md, action.yml and code comments have been updated accordingly. Tests are passing.

If you prefer reviewing less files and compiling the source code yourselves, I have created a parallel PR(#37) containing only the code changes without the built dist files.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@Olfi01
Copy link
Author

Olfi01 commented Jun 12, 2023

@ecraw-amzn Is there something I still need to do before this PR can be reviewed and merged?

@jbct
Copy link

jbct commented Jul 17, 2023

Thanks for the PR, @Olfi01 - we are evaluating this as a part of issue #14

@tvb
Copy link

tvb commented Dec 15, 2023

@jbct can we merge this. Currently, we are facing issues due to the Secrets Cleanup right after the fetch.
Nevermind, the documentation wasn't clear we shouldn't use vars., but use env. instead to access the variables.

@Danny-Smart
Copy link

Is there anything holding this feature up?

@Danny-Smart
Copy link

Bump

We've encountered this issue again. What is the status of the PR?

@Olfi01
Copy link
Author

Olfi01 commented Sep 2, 2024

Bump

We've encountered this issue again. What is the status of the PR?

Since it has been left unresolved for over a year now, some conflicts have mounted up. I don't know if I'll get around to fixing it anytime soon, though I imagine the conflict can't be too difficult to manage.

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

Successfully merging this pull request may close these issues.

4 participants