Fix args for tls cert and key #835
Conversation
tekton-robot
added
the
release-note
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
tekton-robot
added
the
size/XS
| @@ -565,13 +565,13 @@ func getContainer(el *v1alpha1.EventListener) corev1.Container { | |||
| "-el-name", el.Name, | |||
| "-el-namespace", el.Namespace, | |||
| "-port", strconv.Itoa(*ElPort), | |||
| "-tls-cert", elCert, | |||
There was a problem hiding this comment.
this looks like we just changed the position of the args...did that actually solve the issue? 🤯
There was a problem hiding this comment.
I am also confused about this.
There was a problem hiding this comment.
Issue is whatever args we set after -is-multi-ns the args value for those are not effected in initializtion.go file
ex:
Added -port after -is-multi-ns and the port value not effected in initializtion.go file
{"level":"fatal","ts":"2020-11-22T15:47:04.890Z","logger":"eventlistener","caller":"eventlistenersink/main.go:80","msg":"-port arg not found","knative.dev/controller":"eventlistener","stacktrace":"main.main\n\tgithub.com/tektoncd/triggers/cmd/eventlistenersink/main.go:80\nruntime.main\n\truntime/proc.go:204"}
Thats why just moved those args above -is-multi-ns
There was a problem hiding this comment.
Not sure what's the issue with -is-multi-ns 🤔
There was a problem hiding this comment.
got it, I think setting is-multi-ns: "false" is messing things up. Maybe we can just not set the flag when its false and then just set is-multi-ns when we need to set it to true.
There was a problem hiding this comment.
From https://golang.org/pkg/flag/#hdr-Command_line_flag_syntax:
The following forms are permitted:
-flag
-flag=x
-flag x // non-boolean flags only
In the deployment that we generate, we are using the last form (-flag x) which does not work for boolean flags.
There was a problem hiding this comment.
Yes will modify logic for is-multi-ns
There was a problem hiding this comment.
@dibyom That was not the issue. Issue is something else. Even if we set -is-multi-ns to false, I found that certain permutations weren't working. So I decided to change the way we pass Arg and every permutations work.
There was a problem hiding this comment.
With that changes it fixes issue with --is-multi-ns but still we get permission issue
E1123 08:17:43.821688 1 reflector.go:178] k8s.io/client-go@v11.0.1-0.20190805182717-6502b5e7b1b5+incompatible/tools/cache/reflector.go:125: Failed to list *v1alpha1.ClusterTriggerBinding: clustertriggerbindings.triggers.tekton.dev is forbidden: User "system:serviceaccount:tlstest:tekton-triggers-tls-sa" cannot list resource "clustertriggerbindings" in API group "triggers.tekton.dev" at the cluster scope
E1123 08:17:47.527568 1 reflector.go:178] k8s.io/client-go@v11.0.1-0.20190805182717-6502b5e7b1b5+incompatible/tools/cache/reflector.go:125: Failed to list *v1alpha1.ClusterTriggerBinding: clustertriggerbindings.triggers.tekton.dev is forbidden: User "system:serviceaccount:tlstest:tekton-triggers-tls-sa" cannot list resource "clustertriggerbindings" in API group "triggers.tekton.dev" at the cluster scope
E1123 08:17:54.554289 1 reflector.go:178] k8s.io/client-go@v11.0.1-0.20190805182717-6502b5e7b1b5+incompatible/tools/cache/reflector.go:125: Failed to list *v1alpha1.ClusterTriggerBinding: clustertriggerbindings.triggers.tekton.dev is forbidden: User "system:serviceaccount:tlstest:tekton-triggers-tls-sa" cannot list resource "clustertriggerbindings" in API group "triggers.tekton.dev" at the cluster scope
There was a problem hiding this comment.
I will be closing this as its fixed by 838 PR
Changes
Issue:
Fix: cert and key files were not transferred to el main.go and because of that we were getting above error
/assign @dibyom @khrm
Submitter Checklist
These are the criteria that every PR should meet, please check them off as you
review them:
See the contribution guide for more details.
Release Notes