GitHub provider (supersedes #3014)#3115
Conversation
Signed-off-by: Mike Serchenia <michael_serchenia@epam.com>
Signed-off-by: Mike Serchenia <michael_serchenia@epam.com>
Signed-off-by: Mike Serchenia <michael_serchenia@epam.com>
|
|
hi @mike-serchenia, thanks for your contribution. Just to be clear, here the idea is to create app tokens for a given application installation, right? If that's the case, this is better suited to be a |
|
With the current approach, there will never be any two intervals where the data hash will remain the same, this would break some features and assurances we have when it comes to |
|
(I also do not see it as a superseeding PR to #2843, as that one was about pushing to Github actions secrets :) |
Hi! |
|
feel free to update this one 😄 |
|
Still waiting for this with great suspense! |
|
I'll get back to it hopefully next week. |
Signed-off-by: Mike Serchenia <michael_serchenia@epam.com>
Signed-off-by: Mike Serchenia <michael_serchenia@epam.com>
Signed-off-by: Mike Serchenia <michael_serchenia@epam.com>
|
can you check my PR and approve if it looks good? |
pkg/generator/github/github.go
Outdated
| return nil, fmt.Errorf("error creating request: %w", err) | ||
| } | ||
| // Github api expects POST request | ||
| req, err := http.NewRequestWithContext(context.TODO(), "POST", gh.URL, http.NoBody) |
There was a problem hiding this comment.
Pass in the context from generate.
There was a problem hiding this comment.
Replace "POST" with http.MethodPost.
pkg/generator/github/github.go
Outdated
|
|
||
| // git access token | ||
| var gat map[string]interface{} | ||
| if err := json.NewDecoder(resp.Body).Decode(&gat); err != nil && resp.StatusCode < 300 { |
pkg/generator/github/github.go
Outdated
|
|
||
| accessToken, ok := gat["token"].(string) | ||
| if !ok { | ||
| return nil, fmt.Errorf("token is not a string") |
There was a problem hiding this comment.
Technically... token is not a string or token key doesn't exist. :)
pkg/generator/github/github.go
Outdated
| func newGHClient(ctx context.Context, k client.Client, n string, hc *http.Client, | ||
| js *apiextensions.JSON) (*Github, error) { | ||
| if hc == nil { | ||
| hc = &http.Client{} |
There was a problem hiding this comment.
Set up some timeouts.
hc = &http.Server{
ReadTimeout: 5 * time.Second,
WriteTimeout: 10 * time.Second,
}
pkg/generator/github/github.go
Outdated
| gh := &Github{Kube: k, Namespace: n, HTTP: hc} | ||
|
|
||
| ghPath := fmt.Sprintf("/app/installations/%s/access_tokens", res.Spec.InstallID) | ||
| gh.URL = "https://api.github.com" + ghPath |
There was a problem hiding this comment.
This URL should be configurable by the user since this can be enterprise or an on-prem installation.
There was a problem hiding this comment.
Wait a second... This URL should already be configured. Are you overwriting it here? 🤔 Why?
There was a problem hiding this comment.
url can be set by
apis/generators/v1alpha1/generator_github.go:25
type GithubAccessTokenSpec struct {
// URL configures the Github instance URL. Defaults to https://github.com/.
URL string `json:"url,omitempty"`
replaced with:
const (
defaultGithubAPI = "https://api.github.com"
)
gh.URL = defaultGithubAPI + ghPath
if res.Spec.URL != "" {
gh.URL = res.Spec.URL + ghPath
}
| if err != nil || storeBytes == nil { | ||
| return "", fmt.Errorf("failed to marshal store spec: %w", err) | ||
| } | ||
|
|
There was a problem hiding this comment.
this is a great question :), thank you for pointing out
| - "ecrauthorizationtokens" | ||
| - "fakes" | ||
| - "gcraccesstokens" | ||
| - "githubaccesstokens" |
Skarlso
left a comment
There was a problem hiding this comment.
Left you some review comments. :)
Signed-off-by: Mike Serchenia <michael_serchenia@epam.com>
|
Thank you for the comments! Just pushed commit addressing each comment. |
|
LGTM. @gusfcarvalho Is the new generator okay? |
|
@gusfcarvalho could you please check if the PR is okay or some changes are required? |
|
As far as I understand, it looks okay. Let's hope I'm not wrong. :D |
…3115) * github provider signed, supersedes external-secrets#3014 Signed-off-by: Mike Serchenia <michael_serchenia@epam.com> * tests pass, + crd + docs Signed-off-by: Mike Serchenia <michael_serchenia@epam.com> * fix sonarLint alert Signed-off-by: Mike Serchenia <michael_serchenia@epam.com> * refactoring, replace secretStore with generator Signed-off-by: Mike Serchenia <michael_serchenia@epam.com> * cosmetics + tst + lint pass Signed-off-by: Mike Serchenia <michael_serchenia@epam.com> * docs Signed-off-by: Mike Serchenia <michael_serchenia@epam.com> * clean-up + lint + test Signed-off-by: Mike Serchenia <michael_serchenia@epam.com> * small refactor, fix issues left in comments Signed-off-by: Mike Serchenia <michael_serchenia@epam.com> --------- Signed-off-by: Mike Serchenia <michael_serchenia@epam.com> Signed-off-by: Bude8 <henryblee8@gmail.com>
…3115) * github provider signed, supersedes external-secrets#3014 Signed-off-by: Mike Serchenia <michael_serchenia@epam.com> * tests pass, + crd + docs Signed-off-by: Mike Serchenia <michael_serchenia@epam.com> * fix sonarLint alert Signed-off-by: Mike Serchenia <michael_serchenia@epam.com> * refactoring, replace secretStore with generator Signed-off-by: Mike Serchenia <michael_serchenia@epam.com> * cosmetics + tst + lint pass Signed-off-by: Mike Serchenia <michael_serchenia@epam.com> * docs Signed-off-by: Mike Serchenia <michael_serchenia@epam.com> * clean-up + lint + test Signed-off-by: Mike Serchenia <michael_serchenia@epam.com> * small refactor, fix issues left in comments Signed-off-by: Mike Serchenia <michael_serchenia@epam.com> --------- Signed-off-by: Mike Serchenia <michael_serchenia@epam.com> Signed-off-by: Bill Hamilton <bill.hamilton@delinea.com>
I'd like to propose new provider
Github.I uses Github application privateKey and appID to get github
accessTokenof formatghs_*The token can be used to access GH repositories, pull/push to registry, etc.Can be used for tekton auth with GH instead of using personal access tokens'
Checklist
git commit --signoffmake testmake reviewableto replace #3014 with multiple merge conflicts