Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions example/newreposecretwithxcrypto/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,21 +143,21 @@ func addRepoSecret(ctx context.Context, client *github.Client, owner, repo, secr
return nil
}

func encryptSecretWithPublicKey(publicKey *github.PublicKey, secretName, secretValue string) (*github.EncryptedSecret, error) {
func encryptSecretWithPublicKey(publicKey *github.PublicKey, secretName, secretValue string) (github.EncryptedSecret, error) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What value is there to changing the return type from a pointer to a value?
It is pretty standard practice in Go to return nil when there is an error.
Why would we change that for this method?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure it makes any difference in this code if we're returning nil or an empty struct, AFAIK both could be considered idiomatic Go. I did this just before logging off for the weekend so I'm on my phone and can't see if this is a linter error? If it is I'll fix it. FYI I don't this example is particularly idiomatic Go given the unnecessary function separation.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, there are linter and test errors in this PR.

During code reviews at Google, I was told to not return a value struct during an error because a nil value was perfectly valid. So from my experience with the Go team, it is more idiomatic to return a nil pointer to a struct than to return an empty value struct when errors are possible.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I'll fix it up when I get a few mins. I'll probably just get rid of the function call as that'd remove the whole issue.

decodedPublicKey, err := base64.StdEncoding.DecodeString(publicKey.GetKey())
if err != nil {
return nil, fmt.Errorf("base64.StdEncoding.DecodeString was unable to decode public key: %v", err)
return github.EncryptedSecret{}, fmt.Errorf("base64.StdEncoding.DecodeString was unable to decode public key: %v", err)
}

boxKey := [32]byte(decodedPublicKey)
encryptedBytes, err := box.SealAnonymous([]byte{}, []byte(secretValue), &boxKey, crypto_rand.Reader)
if err != nil {
return nil, fmt.Errorf("box.SealAnonymous failed with error %w", err)
return github.EncryptedSecret{}, fmt.Errorf("box.SealAnonymous failed with error %w", err)
}

encryptedString := base64.StdEncoding.EncodeToString(encryptedBytes)
keyID := publicKey.GetKeyID()
encryptedSecret := &github.EncryptedSecret{
encryptedSecret := github.EncryptedSecret{
Name: secretName,
KeyID: keyID,
EncryptedValue: encryptedString,
Expand Down
58 changes: 23 additions & 35 deletions github/actions_secrets.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,11 +85,11 @@ func (s *ActionsService) GetOrgPublicKey(ctx context.Context, org string) (*Publ

// GetEnvPublicKey gets a public key that should be used for secret encryption.
//
// GitHub API docs: https://docs.github.com/enterprise-server@3.7/rest/actions/secrets#get-an-environment-public-key
// GitHub API docs: https://docs.github.com/rest/actions/secrets?apiVersion=2022-11-28#get-an-environment-public-key
//
//meta:operation GET /repositories/{repository_id}/environments/{environment_name}/secrets/public-key
func (s *ActionsService) GetEnvPublicKey(ctx context.Context, repoID int, env string) (*PublicKey, *Response, error) {
url := fmt.Sprintf("repositories/%v/environments/%v/secrets/public-key", repoID, env)
//meta:operation GET /repos/{owner}/{repo}/environments/{environment_name}/secrets/public-key
func (s *ActionsService) GetEnvPublicKey(ctx context.Context, owner, repo, env string) (*PublicKey, *Response, error) {
url := fmt.Sprintf("repos/%v/%v/environments/%v/secrets/public-key", owner, repo, env)
return s.getPublicKey(ctx, url)
}

Expand Down Expand Up @@ -163,11 +163,11 @@ func (s *ActionsService) ListOrgSecrets(ctx context.Context, org string, opts *L

// ListEnvSecrets lists all secrets available in an environment.
//
// GitHub API docs: https://docs.github.com/enterprise-server@3.7/rest/actions/secrets#list-environment-secrets
// GitHub API docs: https://docs.github.com/rest/actions/secrets?apiVersion=2022-11-28#list-environment-secrets
//
//meta:operation GET /repositories/{repository_id}/environments/{environment_name}/secrets
func (s *ActionsService) ListEnvSecrets(ctx context.Context, repoID int, env string, opts *ListOptions) (*Secrets, *Response, error) {
url := fmt.Sprintf("repositories/%v/environments/%v/secrets", repoID, env)
//meta:operation GET /repos/{owner}/{repo}/environments/{environment_name}/secrets
func (s *ActionsService) ListEnvSecrets(ctx context.Context, owner, repo, env string, opts *ListOptions) (*Secrets, *Response, error) {
url := fmt.Sprintf("repos/%v/%v/environments/%v/secrets", owner, repo, env)
return s.listSecrets(ctx, url, opts)
}

Expand Down Expand Up @@ -208,11 +208,11 @@ func (s *ActionsService) GetOrgSecret(ctx context.Context, org, name string) (*S

// GetEnvSecret gets a single environment secret without revealing its encrypted value.
//
// GitHub API docs: https://docs.github.com/enterprise-server@3.7/rest/actions/secrets#get-an-environment-secret
// GitHub API docs: https://docs.github.com/rest/actions/secrets?apiVersion=2022-11-28#get-an-environment-secret
//
//meta:operation GET /repositories/{repository_id}/environments/{environment_name}/secrets/{secret_name}
func (s *ActionsService) GetEnvSecret(ctx context.Context, repoID int, env, secretName string) (*Secret, *Response, error) {
url := fmt.Sprintf("repositories/%v/environments/%v/secrets/%v", repoID, env, secretName)
//meta:operation GET /repos/{owner}/{repo}/environments/{environment_name}/secrets/{secret_name}
func (s *ActionsService) GetEnvSecret(ctx context.Context, owner, repo, env, secretName string) (*Secret, *Response, error) {
url := fmt.Sprintf("repos/%v/%v/environments/%v/secrets/%v", owner, repo, env, secretName)
return s.getSecret(ctx, url)
}

Expand All @@ -232,7 +232,7 @@ type EncryptedSecret struct {
SelectedRepositoryIDs SelectedRepoIDs `json:"selected_repository_ids,omitempty"`
}

func (s *ActionsService) putSecret(ctx context.Context, url string, body *EncryptedSecret) (*Response, error) {
func (s *ActionsService) putSecret(ctx context.Context, url string, body EncryptedSecret) (*Response, error) {
req, err := s.client.NewRequest(ctx, "PUT", url, body)
if err != nil {
return nil, err
Expand All @@ -246,11 +246,7 @@ func (s *ActionsService) putSecret(ctx context.Context, url string, body *Encryp
// GitHub API docs: https://docs.github.com/rest/actions/secrets?apiVersion=2022-11-28#create-or-update-a-repository-secret
//
//meta:operation PUT /repos/{owner}/{repo}/actions/secrets/{secret_name}
func (s *ActionsService) CreateOrUpdateRepoSecret(ctx context.Context, owner, repo string, body *EncryptedSecret) (*Response, error) {
if body == nil {
return nil, errors.New("encrypted secret must be provided")
}

func (s *ActionsService) CreateOrUpdateRepoSecret(ctx context.Context, owner, repo string, body EncryptedSecret) (*Response, error) {
url := fmt.Sprintf("repos/%v/%v/actions/secrets/%v", owner, repo, body.Name)
return s.putSecret(ctx, url, body)
}
Expand All @@ -260,26 +256,18 @@ func (s *ActionsService) CreateOrUpdateRepoSecret(ctx context.Context, owner, re
// GitHub API docs: https://docs.github.com/rest/actions/secrets?apiVersion=2022-11-28#create-or-update-an-organization-secret
//
//meta:operation PUT /orgs/{org}/actions/secrets/{secret_name}
func (s *ActionsService) CreateOrUpdateOrgSecret(ctx context.Context, org string, body *EncryptedSecret) (*Response, error) {
if body == nil {
return nil, errors.New("encrypted secret must be provided")
}

func (s *ActionsService) CreateOrUpdateOrgSecret(ctx context.Context, org string, body EncryptedSecret) (*Response, error) {
url := fmt.Sprintf("orgs/%v/actions/secrets/%v", org, body.Name)
return s.putSecret(ctx, url, body)
}

// CreateOrUpdateEnvSecret creates or updates a single environment secret with an encrypted value.
//
// GitHub API docs: https://docs.github.com/enterprise-server@3.7/rest/actions/secrets#create-or-update-an-environment-secret
// GitHub API docs: https://docs.github.com/rest/actions/secrets?apiVersion=2022-11-28#create-or-update-an-environment-secret
//
//meta:operation PUT /repositories/{repository_id}/environments/{environment_name}/secrets/{secret_name}
func (s *ActionsService) CreateOrUpdateEnvSecret(ctx context.Context, repoID int, env string, body *EncryptedSecret) (*Response, error) {
if body == nil {
return nil, errors.New("encrypted secret must be provided")
}

url := fmt.Sprintf("repositories/%v/environments/%v/secrets/%v", repoID, env, body.Name)
//meta:operation PUT /repos/{owner}/{repo}/environments/{environment_name}/secrets/{secret_name}
func (s *ActionsService) CreateOrUpdateEnvSecret(ctx context.Context, owner, repo, env string, body EncryptedSecret) (*Response, error) {
url := fmt.Sprintf("repos/%v/%v/environments/%v/secrets/%v", owner, repo, env, body.Name)
return s.putSecret(ctx, url, body)
}

Expand Down Expand Up @@ -314,11 +302,11 @@ func (s *ActionsService) DeleteOrgSecret(ctx context.Context, org, name string)

// DeleteEnvSecret deletes a secret in an environment using the secret name.
//
// GitHub API docs: https://docs.github.com/enterprise-server@3.7/rest/actions/secrets#delete-an-environment-secret
// GitHub API docs: https://docs.github.com/rest/actions/secrets?apiVersion=2022-11-28#delete-an-environment-secret
//
//meta:operation DELETE /repositories/{repository_id}/environments/{environment_name}/secrets/{secret_name}
func (s *ActionsService) DeleteEnvSecret(ctx context.Context, repoID int, env, secretName string) (*Response, error) {
url := fmt.Sprintf("repositories/%v/environments/%v/secrets/%v", repoID, env, secretName)
//meta:operation DELETE /repos/{owner}/{repo}/environments/{environment_name}/secrets/{secret_name}
func (s *ActionsService) DeleteEnvSecret(ctx context.Context, owner, repo, env, secretName string) (*Response, error) {
url := fmt.Sprintf("repos/%v/%v/environments/%v/secrets/%v", owner, repo, env, secretName)
return s.deleteSecret(ctx, url)
}

Expand Down
54 changes: 27 additions & 27 deletions github/actions_secrets_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,7 @@ func TestActionsService_CreateOrUpdateRepoSecret(t *testing.T) {
t.Parallel()
client, mux, _ := setup(t)

input := &EncryptedSecret{
input := EncryptedSecret{
Name: "NAME",
EncryptedValue: "QIv=",
KeyID: "1234",
Expand All @@ -315,7 +315,7 @@ func TestActionsService_CreateOrUpdateRepoSecret(t *testing.T) {

const methodName = "CreateOrUpdateRepoSecret"
testBadOptions(t, methodName, func() (err error) {
_, err = client.Actions.CreateOrUpdateRepoSecret(ctx, "o", "r", nil)
_, err = client.Actions.CreateOrUpdateRepoSecret(ctx, "o", "r", EncryptedSecret{})
return err
})
testBadOptions(t, methodName, func() (err error) {
Expand Down Expand Up @@ -477,7 +477,7 @@ func TestActionsService_CreateOrUpdateOrgSecret(t *testing.T) {
t.Parallel()
client, mux, _ := setup(t)

input := &EncryptedSecret{
input := EncryptedSecret{
Name: "NAME",
EncryptedValue: "QIv=",
KeyID: "1234",
Expand Down Expand Up @@ -506,7 +506,7 @@ func TestActionsService_CreateOrUpdateOrgSecret(t *testing.T) {

const methodName = "CreateOrUpdateOrgSecret"
testBadOptions(t, methodName, func() (err error) {
_, err = client.Actions.CreateOrUpdateOrgSecret(ctx, "o", nil)
_, err = client.Actions.CreateOrUpdateOrgSecret(ctx, "o", EncryptedSecret{})
return err
})
testBadOptions(t, methodName, func() (err error) {
Expand Down Expand Up @@ -682,13 +682,13 @@ func TestActionsService_GetEnvPublicKey(t *testing.T) {
t.Parallel()
client, mux, _ := setup(t)

mux.HandleFunc("/repositories/1/environments/e/secrets/public-key", func(w http.ResponseWriter, r *http.Request) {
mux.HandleFunc("/repos/o/r/environments/e/secrets/public-key", func(w http.ResponseWriter, r *http.Request) {
testMethod(t, r, "GET")
fmt.Fprint(w, `{"key_id":"1234","key":"2Sg8iYjAxxmI2LvUXpJjkYrMxURPc8r+dB7TJyvv1234"}`)
})

ctx := t.Context()
key, _, err := client.Actions.GetEnvPublicKey(ctx, 1, "e")
key, _, err := client.Actions.GetEnvPublicKey(ctx, "o", "r", "e")
if err != nil {
t.Errorf("Actions.GetEnvPublicKey returned error: %v", err)
}
Expand All @@ -700,12 +700,12 @@ func TestActionsService_GetEnvPublicKey(t *testing.T) {

const methodName = "GetEnvPublicKey"
testBadOptions(t, methodName, func() (err error) {
_, _, err = client.Actions.GetEnvPublicKey(ctx, 0.0, "\n")
_, _, err = client.Actions.GetEnvPublicKey(ctx, "\n", "\n", "\n")
return err
})

testNewRequestAndDoFailure(t, methodName, client, func() (*Response, error) {
got, resp, err := client.Actions.GetEnvPublicKey(ctx, 1, "e")
got, resp, err := client.Actions.GetEnvPublicKey(ctx, "o", "r", "e")
if got != nil {
t.Errorf("testNewRequestAndDoFailure %v = %#v, want nil", methodName, got)
}
Expand All @@ -723,7 +723,7 @@ func TestActionsService_GetEnvPublicKeyNumeric(t *testing.T) {
})

ctx := t.Context()
key, _, err := client.Actions.GetEnvPublicKey(ctx, 1, "e")
key, _, err := client.Actions.GetEnvPublicKey(ctx, "o", "r", "e")
if err != nil {
t.Errorf("Actions.GetEnvPublicKey returned error: %v", err)
}
Expand All @@ -735,12 +735,12 @@ func TestActionsService_GetEnvPublicKeyNumeric(t *testing.T) {

const methodName = "GetEnvPublicKey"
testBadOptions(t, methodName, func() (err error) {
_, _, err = client.Actions.GetEnvPublicKey(ctx, 0.0, "\n")
_, _, err = client.Actions.GetEnvPublicKey(ctx, "\n", "\n", "\n")
return err
})

testNewRequestAndDoFailure(t, methodName, client, func() (*Response, error) {
got, resp, err := client.Actions.GetEnvPublicKey(ctx, 1, "e")
got, resp, err := client.Actions.GetEnvPublicKey(ctx, "o", "r", "e")
if got != nil {
t.Errorf("testNewRequestAndDoFailure %v = %#v, want nil", methodName, got)
}
Expand All @@ -752,15 +752,15 @@ func TestActionsService_ListEnvSecrets(t *testing.T) {
t.Parallel()
client, mux, _ := setup(t)

mux.HandleFunc("/repositories/1/environments/e/secrets", func(w http.ResponseWriter, r *http.Request) {
mux.HandleFunc("/repos/o/r/environments/e/secrets", func(w http.ResponseWriter, r *http.Request) {
testMethod(t, r, "GET")
testFormValues(t, r, values{"per_page": "2", "page": "2"})
fmt.Fprint(w, `{"total_count":4,"secrets":[{"name":"A","created_at":`+refTimeStr(1136178000)+`,"updated_at":`+refTimeStr(1136178001)+`},{"name":"B","created_at":`+refTimeStr(1136178002)+`,"updated_at":`+refTimeStr(1136178003)+`}]}`)
})

opts := &ListOptions{Page: 2, PerPage: 2}
ctx := t.Context()
secrets, _, err := client.Actions.ListEnvSecrets(ctx, 1, "e", opts)
secrets, _, err := client.Actions.ListEnvSecrets(ctx, "o", "r", "e", opts)
if err != nil {
t.Errorf("Actions.ListEnvSecrets returned error: %v", err)
}
Expand All @@ -778,12 +778,12 @@ func TestActionsService_ListEnvSecrets(t *testing.T) {

const methodName = "ListEnvSecrets"
testBadOptions(t, methodName, func() (err error) {
_, _, err = client.Actions.ListEnvSecrets(ctx, 0.0, "\n", opts)
_, _, err = client.Actions.ListEnvSecrets(ctx, "\n", "\n", "\n", opts)
return err
})

testNewRequestAndDoFailure(t, methodName, client, func() (*Response, error) {
got, resp, err := client.Actions.ListEnvSecrets(ctx, 1, "e", opts)
got, resp, err := client.Actions.ListEnvSecrets(ctx, "o", "r", "e", opts)
if got != nil {
t.Errorf("testNewRequestAndDoFailure %v = %#v, want nil", methodName, got)
}
Expand All @@ -795,13 +795,13 @@ func TestActionsService_GetEnvSecret(t *testing.T) {
t.Parallel()
client, mux, _ := setup(t)

mux.HandleFunc("/repositories/1/environments/e/secrets/secret", func(w http.ResponseWriter, r *http.Request) {
mux.HandleFunc("/repos/o/r/environments/e/secrets/secret", func(w http.ResponseWriter, r *http.Request) {
testMethod(t, r, "GET")
fmt.Fprint(w, `{"name":"secret","created_at":`+refTimeStr(1136178000)+`,"updated_at":`+refTimeStr(1136178001)+`}`)
})

ctx := t.Context()
secret, _, err := client.Actions.GetEnvSecret(ctx, 1, "e", "secret")
secret, _, err := client.Actions.GetEnvSecret(ctx, "o", "r", "e", "secret")
if err != nil {
t.Errorf("Actions.GetEnvSecret returned error: %v", err)
}
Expand All @@ -817,12 +817,12 @@ func TestActionsService_GetEnvSecret(t *testing.T) {

const methodName = "GetEnvSecret"
testBadOptions(t, methodName, func() (err error) {
_, _, err = client.Actions.GetEnvSecret(ctx, 0.0, "\n", "\n")
_, _, err = client.Actions.GetEnvSecret(ctx, "\n", "\n", "\n", "\n")
return err
})

testNewRequestAndDoFailure(t, methodName, client, func() (*Response, error) {
got, resp, err := client.Actions.GetEnvSecret(ctx, 1, "e", "secret")
got, resp, err := client.Actions.GetEnvSecret(ctx, "o", "r", "e", "secret")
if got != nil {
t.Errorf("testNewRequestAndDoFailure %v = %#v, want nil", methodName, got)
}
Expand All @@ -834,7 +834,7 @@ func TestActionsService_CreateOrUpdateEnvSecret(t *testing.T) {
t.Parallel()
client, mux, _ := setup(t)

input := &EncryptedSecret{
input := EncryptedSecret{
Name: "secret",
EncryptedValue: "QIv=",
KeyID: "1234",
Expand All @@ -852,23 +852,23 @@ func TestActionsService_CreateOrUpdateEnvSecret(t *testing.T) {
})

ctx := t.Context()
_, err := client.Actions.CreateOrUpdateEnvSecret(ctx, 1, "e", input)
_, err := client.Actions.CreateOrUpdateEnvSecret(ctx, "o", "r", "e", input)
if err != nil {
t.Errorf("Actions.CreateOrUpdateEnvSecret returned error: %v", err)
}

const methodName = "CreateOrUpdateEnvSecret"
testBadOptions(t, methodName, func() (err error) {
_, err = client.Actions.CreateOrUpdateEnvSecret(ctx, 1, "e", nil)
_, err = client.Actions.CreateOrUpdateEnvSecret(ctx, "\n", "\n", "\n", EncryptedSecret{})
return err
})
testBadOptions(t, methodName, func() (err error) {
_, err = client.Actions.CreateOrUpdateEnvSecret(ctx, 0.0, "\n", input)
_, err = client.Actions.CreateOrUpdateEnvSecret(ctx, "\n", "\n", "\n", input)
return err
})

testNewRequestAndDoFailure(t, methodName, client, func() (*Response, error) {
return client.Actions.CreateOrUpdateEnvSecret(ctx, 1, "e", input)
return client.Actions.CreateOrUpdateEnvSecret(ctx, "\n", "\n", "\n", input)
})
}

Expand All @@ -881,18 +881,18 @@ func TestActionsService_DeleteEnvSecret(t *testing.T) {
})

ctx := t.Context()
_, err := client.Actions.DeleteEnvSecret(ctx, 1, "e", "secret")
_, err := client.Actions.DeleteEnvSecret(ctx, "o", "r", "e", "secret")
if err != nil {
t.Errorf("Actions.DeleteEnvSecret returned error: %v", err)
}

const methodName = "DeleteEnvSecret"
testBadOptions(t, methodName, func() (err error) {
_, err = client.Actions.DeleteEnvSecret(ctx, 0.0, "\n", "\n")
_, err = client.Actions.DeleteEnvSecret(ctx, "\n", "\n", "\n", "\n")
return err
})

testNewRequestAndDoFailure(t, methodName, client, func() (*Response, error) {
return client.Actions.DeleteEnvSecret(ctx, 1, "r", "secret")
return client.Actions.DeleteEnvSecret(ctx, "\n", "\n", "\n", "\n")
})
}
4 changes: 2 additions & 2 deletions github/github-iterators.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading
Loading