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

Fix support for uppercase environment variables #717

Open
wants to merge 4 commits into
base: main-enterprise
Choose a base branch
from

Conversation

klutchell
Copy link
Contributor

Github REST API supports case-sensitive names on this endpoint
so we should always create (POST) variables with the desired case.

On subsequent GET, PATCH, and DELETE requests the Github REST API
will use loose matching to find any existing objects, ignoring case.

So in general we need to force to lowercase to perform diffs, and respect
the provided case for create, but for all other calls it doesn't matter
which case is used.

@klutchell
Copy link
Contributor Author

FYI @decyjphr @luvsaxena1 , another couple fixes for environment variables.

@klutchell klutchell force-pushed the kyle/uppercase-env-vars branch from 2b1f9f7 to 523dd73 Compare December 22, 2024 16:58
Signed-off-by: Kyle Harding <[email protected]>
This boolean was reversed when filtering existing variables,
so it consistently sent a POST request to existing vars.

Signed-off-by: Kyle Harding <[email protected]>
@klutchell klutchell force-pushed the kyle/uppercase-env-vars branch from 523dd73 to dce0116 Compare December 26, 2024 17:13
The current behaviour was to force all variable names
to lowercase which was resulting in the wrong behaviour on POST.

Instead, just force lowercase when comparing with existing variables.

Signed-off-by: Kyle Harding <[email protected]>
@klutchell klutchell force-pushed the kyle/uppercase-env-vars branch 2 times, most recently from 5df3314 to d46d969 Compare December 26, 2024 20:11
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.

1 participant