-
Notifications
You must be signed in to change notification settings - Fork 927
feat: add environment secret import capabilities #2462
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
base: main
Are you sure you want to change the base?
feat: add environment secret import capabilities #2462
Conversation
|
changes are based off of #1763 |
|
Would love to see this merged instead of having to patch the Terrraform state directly. Please. |
|
I just tried building the current 6.6.0 provider with this code merged in so I can do some importing. Not a go guru, but I think I am using go 1.22.4 on linux. I get an error I was able to get around it by adding |
|
@dcfsc Thanks for pointing that out. I just pushed the update |
|
@dcfsc This PR is pretty old, I am not sure how to get the attention of the code admins to approve the CI checks. |
Me either. At least I can hack the provider for the cases I need it. Not an optimal solution, to be sure. Thanks for the fix! |
| owner := meta.(*Owner).name | ||
| ctx := context.Background() | ||
|
|
||
| parts := strings.Split(d.Id(), "/") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you use colon for the import separator? Then you could use parseID3 here
| func resourceGithubActionsEnvironmentSecretImport(d *schema.ResourceData, meta interface{}) ([]*schema.ResourceData, error) { | ||
| client := meta.(*Owner).v3client | ||
| owner := meta.(*Owner).name | ||
| ctx := context.Background() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you use Context-aware provider functions instead?
| return nil, err | ||
| } | ||
|
|
||
| repo, _, err := client.Repositories.Get(ctx, owner, repoName) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This shouldn't be necessary if the import ID contains enough information to be able to successfully run Read afterwards.
Usually that's the ID
| } | ||
| `, randomID, envName, secretName, secretValue) | ||
|
|
||
| testCase := func(t *testing.T, mode string) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We've abandoned this pattern in tests, could you conform to the new pattern?
Resolves #2461
Before the change?
After I run an import command
After the change?
I should be able to run:
or use and import block
Pull request checklist
Does this introduce a breaking change?
No
Please see our docs on breaking changes to help!