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

[#803] Allow GitHub App PEM data to be passed directly #804

Merged
merged 1 commit into from
Jun 8, 2021

Conversation

jspiro
Copy link
Contributor

@jspiro jspiro commented May 27, 2021

This is a generic (but breaking) solution to #803 that supports the current functionality via Terraform's file function to emulate previous behavior (wherein you provide a path to a PEM file).

This is cleaner and simpler than providing more named options for file contents, and the original behavior has only been around for a few days or weeks, so it's a good time to break it (and it's trivial to emulate with file()).

We've tested it with our real workspace in Terraform Cloud. It's also the only way we can use this feature, because that private key cannot be secured any other way and has root access to everything.

@gusnuf
Copy link

gusnuf commented May 27, 2021

I like it, definitely not backwards compatible but the feature was only released last Friday.

@jspiro
Copy link
Contributor Author

jspiro commented May 27, 2021

Also a reminder we need a maintainer to approve Actions to run.

@jspiro
Copy link
Contributor Author

jspiro commented May 27, 2021

And it would be awesome if someone wanted to write docs on how to make the GitHub Application for this. We hacked ours together and got it working, but had to give it perms to everything since we weren't sure what was absolutely needed or not needed–can't say we understand it well enough to document it.

@jcudit jcudit added this to the v5.0.0 milestone May 28, 2021
@jspiro
Copy link
Contributor Author

jspiro commented Jun 3, 2021

@jcudit I see this is in the 5.0 milestone–when is that targeted to land?

Might this not be better to merge in v4.11.0 before too many people use the existing method of PEM, since it is not backwards-compatible, and the old method has only been out two weeks? The changes are very simple and more general than the original approach, while still supporting the original approach trivially.

@jcudit jcudit modified the milestones: v5.0.0, v4.11.0 Jun 3, 2021
@jcudit
Copy link
Contributor

jcudit commented Jun 3, 2021

Agreed, lets run with v4.11.0 and hope that friction is minimal when releasing breaking changes in a minor version 🤞🏾

@jcudit jcudit mentioned this pull request Jun 3, 2021
@jspiro
Copy link
Contributor Author

jspiro commented Jun 4, 2021

Cool, thanks for everyone's eyes and understanding! We're very much looking forward to using this too :-)

As this is my first contribution, I'm assuming you'll handle the merge when you're ready! 🖖🏻

@jcudit jcudit merged commit cfd2861 into integrations:master Jun 8, 2021
jcudit pushed a commit that referenced this pull request Jun 15, 2021
jcudit pushed a commit that referenced this pull request Jun 16, 2021
@jodok
Copy link

jodok commented Aug 10, 2021

can someone point me to the documentation on how to set the environment variable for GITHUB_APP_PEM_FILE in terraform cloud? i've tried without newlines, quoted, without the begin and end line,... but always receive an │ Error: No decodeable PEM data found error.

@jspiro
Copy link
Contributor Author

jspiro commented Sep 21, 2021

can someone point me to the documentation on how to set the environment variable for GITHUB_APP_PEM_FILE in terraform cloud? i've tried without newlines, quoted, without the begin and end line,... but always receive an │ Error: No decodeable PEM data found error.

It looks like someone removed my documentation for this: https://github.com/integrations/terraform-provider-github/pull/804/files#diff-4839085b71de83d7294472f5f8fa6652faf4b0b24df211c8eaf945ac96933aa5R109

As for setting it, what have you tried? Can you try it with a variable first as above?

@shrink
Copy link
Contributor

shrink commented Oct 7, 2021

@jspiro are you using a Terraform Variable in Terraform Cloud, or an Environment Variable? As far as I can tell, it's impossible to use GITHUB_APP_PEM_FILE with Terraform Cloud at the moment, so I think you're actually using a Terraform Variable (i.e: pem_file = var...) -- can you confirm?

@jodok I've had the same issue and as far as I know there's no userland workaround, but I've created a PR for what I think will fix it: #931

@jspiro
Copy link
Contributor Author

jspiro commented Oct 7, 2021

@shrink We use a variable, just as you describe, in Terraform Cloud.

@jspiro jspiro deleted the jono/2021-05-26/pem-file-stuff branch October 7, 2021 02:08
kfcampbell pushed a commit to kfcampbell/terraform-provider-github that referenced this pull request Jul 26, 2022
@hargut
Copy link

hargut commented Oct 6, 2023

can someone point me to the documentation on how to set the environment variable for GITHUB_APP_PEM_FILE in terraform cloud? i've tried without newlines, quoted, without the begin and end line,... but always receive an │ Error: No decodeable PEM data found error.

Have seen the same problem, needed to use \\n within the GITHUB_APP_PEM_FILE environment instead of \n to get it working.

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.

6 participants