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

Support variables in environments #711

Open
rlmartin opened this issue Feb 23, 2023 · 4 comments
Open

Support variables in environments #711

rlmartin opened this issue Feb 23, 2023 · 4 comments

Comments

@rlmartin
Copy link
Contributor

Prerequisites:

New Feature

Please describe the desired new functionality:
Under the environments section, add a vars section that is a map of var name => var value. Example:

environments:
  - name: production
    vars:
      AWS_DEPLOYMENT_ROLE: arn:aws:iam::0000000000:role/deployment-role
  - name: development
    vars:
      AWS_DEPLOYMENT_ROLE: arn:aws:iam::1111111111:role/deployment-role
@travi
Copy link
Member

travi commented Mar 3, 2023

i'm supportive of this addition. i would like to see us avoid abbreviations, so i'd prefer variables over vars, but this otherwise seems reasonable. PRs welcome. I will be stepping away for a couple of weeks, but will catch up afterward.

@ctoestreich
Copy link

Any movement on this?

@ctoestreich
Copy link

@rlmartin I took a stab at getting the support in place, but the ability to add environment varaibles required the repository id not the name so I had to add an extra call to get this.

@travi It does not look like the repository id is used anywhere else and the context for what is handed into the app is unclear if this is already on the this.repo object as something like this.repo.id. If that is the case it simplifies the code a lot and I will not have to make the additional call to get the id.

If the call is needed then I need a bit of help fixing the mock as my jest skills are a bit rusty and I can't seem to figure out why the mock to the get repository isn't working. 🤷

@travi
Copy link
Member

travi commented Mar 8, 2024

sorry for the delay responding on this one. i was working on refactoring the integration tests and upgrading to the latest probot version, so i didnt want my feedback to change after working through that effort.

the ability to add environment varaibles required the repository id not the name so I had to add an extra call to get this

can you point to documentation related to this? this doesnt align with what i'm finding. for example, this doc highlights that the repository name is needed, not the id: https://docs.github.com/en/rest/actions/variables?apiVersion=2022-11-28#create-an-environment-variable

the context for what is handed into the app is unclear if this is already on the this.repo object as something like this.repo.id.

the repo data populated in this.repo comes from the webhook payload that is received in our listeners. https://github.com/octokit/webhooks/ is a good reference for the expected content of these payloads. for example, here is an example of how repository is represented in a push webhook.

I need a bit of help fixing the mock as my jest skills are a bit rusty and I can't seem to figure out why the mock to the get repository isn't working.

i agree that the previous testing approach was a bit awkward, so i used the upgrade to the latest probot version as an excuse to rework the approach for integration testing. use of cucumber enabled making the scenarios a lot easier to describe and understand when approaching the project without the whole thing already in your head. hopefully this helps make things more clear, but let me know if additional guidance would be helpful

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

No branches or pull requests

3 participants