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

Enterprise github repo: not generating events and unable to write commit status to github #161

Closed
sairamvoggu1007 opened this issue Mar 16, 2021 · 24 comments · Fixed by #162

Comments

@sairamvoggu1007
Copy link

Team,
version using : flux2 v0.8.0
events are not generating and unable to write commit status to github about the deployment

Following is error noticing in notification controller log.. It was supposed to fetch from my github domain but it's trying to fetch from api.github.com please check following error
**```

{"level":"error","ts":"2021-03-12T18:40:29.577Z","logger":"event-server","msg":"failed to send notification","object":"platform/badlybehavingtestservice","kind":"Kustomizatio │
│ n","error":"could not list commit statuses: Get "https://api.github.com/repos/s-voggu/eks/commits/d1cc91e17c740a4d203e8eef04b8312bab47f46b/statuses?per_page=50\": net/http:



In gitrepository type resource I'm using ssh repo url and able to fetch latest commit status...In providers I'm using https repo url in address field.  In gitrespositry resource and kusomization resource events are getting generated but I don't see any events in alerts and providers . Observing above error in notification controller log please assist asap
@simongottschlag
Copy link

Hi!

There seems to be some missing information in the log you posted after net/http.

Can you confirm the you are able to access the api from a pod and nothing is blocking it?

@sairamvoggu1007
Copy link
Author

sairamvoggu1007 commented Mar 16, 2021

please find my response inline. 1. There seems to be some missing information in the log you posted after net/http. (complete log )

{"level":"error","ts":"2021-03-16T02:22:48.760Z","logger":"event-server","msg":"failed to send notification","object":"platform/orderstatusfeedservice","kind":"Kustomization","error":"could not list commit statuses: Get \"https://api.github.com/repos/s-voggu/eks/commits/mycommitid/statuses?per_page=50\": net/http: invalid header field value \"Bearer for key Authorization"}...

  1. Can you confirm the you are able to access the api from a pod and nothing is blocking it?
    on running pod when I curl api.github.com getting following
sh-4.2$ curl api.github.com
^C
sh-4.2$ curl https://api.github.com
curl: (7) Failed connect to api.github.com:443; Connection timed out

when I hit my github domain... able to succesfully handshake and redirect.
sh-4.2$ curl https://github.pie.com/

You are being redirected.

@simongottschlag
Copy link

Hi,

Please revoke that API key ASAP.

It seems like the formatting of the secret is wrong here and you need to fix that before you can use it with the notification controller.

@sairamvoggu1007
Copy link
Author

sairamvoggu1007 commented Mar 16, 2021

may I know which secret are you referring?

Docs suggesting to encode secrets with base64 as per doc I encoded and created secrets and referring them

@simongottschlag
Copy link

Remember that even though you've edited the comment, it is possible to view the history of the comment. Make sure it is revoked and you use a new one.

First of all, if you are unable to access the GitHub API from a pod in the cluster with curl, then it won't work with notification-controller. Make sure it is working before you continue troubleshooting it.

Secondly, the format of your token seemed to have weird encoding. Try looking at your secret (decoding it) and see what the output is. If you have stuff like \x in it, then there may be something wrong with how you copied it (maybe zero char whitespace).

@simongottschlag
Copy link

Are you using the on-prem version of GitHub and trying to communicate with that API?

@sairamvoggu1007
Copy link
Author

Yes onprem github. we do not want to reach out to api.github.com

@simongottschlag
Copy link

Ah! I believe (but not sure) the notification-controller doesn't have support for that yet.

Create a feature request for it here: https://github.com/fluxcd/notification-controller

Most likely not something hard, but whoever builds it needs access to GitHub on-prem to validate.

@simongottschlag
Copy link

After looking at the code, it seems like the following needs to be used for GitHub Enterprise: https://github.com/google/go-github/blob/51f17753badd669f62e5cc9ead041328e2087f4e/github/github.go#L309

@sairamvoggu1007
Copy link
Author

Sure, I will raise a feature request as mentioned above

@sairamvoggu1007 sairamvoggu1007 changed the title corporate github repo: not generating events and unable to write commit status to github Enterprise github repo: not generating events and unable to write commit status to github Mar 16, 2021
@Nicklason
Copy link

This is not a problem with Github Enterprise, it is a problem with the Kubernetes secret they made. It is because the token is not base64 encoded.

You can use the command below to encode the token.

echo -n "token" | base64 -w 0

@sairamvoggu1007
Copy link
Author

Actually, few days ago already I tried encoding the token and created the secret and referring it. In that case it was not able to fetch the latest commit. I can retry now again and will share the log

@sairamvoggu1007
Copy link
Author

sairamvoggu1007 commented Mar 16, 2021

the log before encoding the git token

{"level":"error","ts":"2021-03-16T02:22:48.760Z","logger":"event-server","msg":"failed to send notification","object":"platform/orderstatusfeedservice","kind":"Kustomization","error":"could not list commit statuses: Get "https://api.github.com/repos/s-voggu/eks/commits/17e02455855f2a055e4d6c998cf8257c87d052b7/statuses?per_page=50\": net/http: invalid header field value "Bearer for key Authorization"}...

the log after encoding the git token

{"level":"error","ts":"2021-03-16T21:55:53.219Z","logger":"event-server","msg":"failed to send notification","object":"platform/orderstatusfeedservice","kind" │ │ :"Kustomization","error":"could not list commit statuses: context deadline exceeded"}

I just decoded the token and it looks good

If we notice the log before encoding at least it was trying to get my repo name and latest commit id but after encoding the token not able to even see or get repo name and latest commit id

@simongottschlag
Copy link

Hi!

Now when the token has the correct encoding, we're hitting the issue with GitHub Enterprise not being supported by the notification-controller.

I do recommend creating the feature request for the notification-controller 😊👍

@phillebaba
Copy link
Member

I can probably answer why this is not working for GitHub enterprise. The reason is most likely because the provider was never built with self hosted Github in mind. If you look at the part that initiates the Github client it ignores the host.

tc := oauth2.NewClient(ctx, ts)

Instead the client needs to be specifically created for Github enterprise.
https://github.com/google/go-github/blob/51f17753badd669f62e5cc9ead041328e2087f4e/github/github.go#L309

Thanks @simongottschlag for filling me in, it helped getting all the details. I can create a PR which adds support for GH enterprise if you @sairamvoggu1007 can test it for me.

@phillebaba phillebaba transferred this issue from fluxcd/flux2 Mar 16, 2021
@phillebaba
Copy link
Member

I moved this issue to the NC so you do not need to create a new issue @sairamvoggu1007

@sairamvoggu1007
Copy link
Author

Yes, please let me know once this is done I can test it.. currently we are on flux2 version0.8.0 @phillebaba

@simongottschlag
Copy link

Thank you @phillebaba for looking at it 😊🙏

@phillebaba
Copy link
Member

phillebaba commented Mar 16, 2021

@sairamvoggu1007 I created a draft PR to track any code comments.
#162

Just checkout the branch and build+push the image to some image registry. Then replace the image used for notification-controller in your "fleet-infra" repository. I do not know if there are any major CRD changes between 0.8 and 0.9 that would complicate testing this, try and see if it works.

@sairamvoggu1007
Copy link
Author

I will test it and let you know once it's done. thanks for the prompt response

@sairamvoggu1007
Copy link
Author

sairamvoggu1007 commented Mar 17, 2021

@phillebaba we built the image. I just need to replace the following image with the image I built right to test the changes?

              fieldPath: metadata.namespace
        image: ghcr.io/fluxcd/notification-controller:v0.9.0
        imagePullPolicy: IfNotPresent

@phillebaba
Copy link
Member

Yep that is the one :)

@sairamvoggu1007
Copy link
Author

@phillebaba unfortunately we don't have any environment open for testing so we can't test this from our side. Could you please let us know once this version is released. thanks

@phillebaba
Copy link
Member

I cant really in good faith submit a PR and release something which I do not know works. You do not need a full cluster to test this, it would work if you install flux on a kind cluster locally on your computer which pulls from your GitHub instance. Could you do that?

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 a pull request may close this issue.

4 participants