Add Istio user agent to kube-client used in istioctl and other binaries #33120

mandarjog · 2021-05-26T15:51:55Z

Add istio-user-agent string to api server calls from istio for auditability.

resolves #28231

Use pkg/version to generation version string for user-agent.
Consolidate multiple paths to build a kubeclient.
Add klog support for client-go. The standard -v=x option cannot be used because we use it elsewhere. istioctl renames the option, and makes it a long form option with --vklog.

--

Following is output with --vkog=9. This flag was also added as part of this PR. shows -H "User-Agent: istioctl/1.11-dev"

jog@dev3:~/go/src/istio.io/istio$ ./out/linux_amd64/istioctl version -vklog=9
I0608 07:11:00.050509  250705 loader.go:372] Config loaded from file:  /home/mjog/.kube/config
I0608 07:11:00.051002  250705 loader.go:372] Config loaded from file:  /home/mjog/.kube/config
I0608 07:11:00.052499  250705 round_trippers.go:435] curl -k -v -XGET  -H "Accept: application/json, */*" -H "User-Agent: istioctl/1.11-dev" 'https://34.67.144.238/api/v1/namespaces/istio-system/pods?fieldSelector=status.phase%3DRunning&labelSelector=app%3Distiod&timeout=10m0s'
I0608 07:11:00.180675  250705 round_trippers.go:454] GET https://34.67.144.238/api/v1/namespaces/istio-system/pods?fieldSelector=status.phase%3DRunning&labelSelector=app%3Distiod&timeout=10m0s 200 OK in 128 milliseconds
I0608 07:11:00.180739  250705 round_trippers.go:460] Response Headers:
I0608 07:11:00.180774  250705 round_trippers.go:463]     Cache-Control: no-cache, private
I0608 07:11:00.180805  250705 round_trippers.go:463]     Content-Type: application/json
I0608 07:11:00.180834  250705 round_trippers.go:463]     Date: Tue, 08 Jun 2021 07:11:00 GMT
I0608 07:11:00.180838  250705 round_trippers.go:463]     Audit-Id: 1c8eebbc-0860-4ac0-8f9e-278b54c8bd8c
I0608 07:11:00.213594  250705 request.go:1123] Response Body {...}

howardjohn

looks good

howardjohn · 2021-05-26T18:26:57Z

operator/cmd/mesh/upgrade.go

@@ -443,7 +444,7 @@ type ExecClient interface {

 // NewClient is the constructor for the client wrapper
 func NewClient(kubeconfig, configContext string) (*Client, error) {
-	config, err := defaultRestConfig(kubeconfig, configContext)
+	config, err := kube.DefaultRestConfig(kubeconfig, configContext)


seems simpler change but should it be in another PR?

Oh nevermind, i see we are setting user agent at a very low level

mandarjog · 2021-06-09T07:06:10Z

/test gencheck_istio

howardjohn

LGTM except comments on the vklog stuff. I think its awesome but may need some tweaks

howardjohn · 2021-06-09T23:30:42Z

istioctl/cmd/istioctl/main.go

+// EnableKlogWithCobra enables klog to work with cobra / pflags.
+// k8s libraries like client-go use klog.
+func EnableKlogWithCobra() {
+	klog.InitFlags(nil)


does passing nil (which then uses goflag.CommandLine) cause any issues here in help messages, etc? Should we create a dedicated FlagSet for this so it doesn't unexpectedly leak out?

howardjohn · 2021-06-09T23:33:02Z

istioctl/cmd/istioctl/main.go

+			Usage:    gf.Usage + ". Like -v flag. ex: --vklog=9",
+		}))
+
+	klog.SetLogger(nil)


If I understand the situation correctly:

Today we can enable "debug" logs from klog with --log_output_level=klog:debug. This uses our logging system (json, stackdriver, etc). But it uses the default klog verbosity level which is not 9.

This pr allows setting the klog verbosity, but it doesn't use our logging system. I think if you remove this line it will allow --vklog to work, but it also requires --log_output_level=klog:debug. It seems preferable to use our logging system, but maybe we can improve the UX

I feel like this is something that is generally useful outside of istioctl - I have often wanted this in istiod. Maybe we should make it so it can be configured as part of istio/pkg? Not sure the UX we want here though (new flag, --log_output_level=klog:debug for most cases and env var to tune -v level, etc).

yeah, once we have it in istioctl, we will add it to add binaries.
In a previous version of the PR I had enabled it by mistake on istiod and is spews logs, but it is very useful.

Can we at least remove klog.SetLogger(nil) so it uses our logging system?

howardjohn · 2021-06-10T00:49:30Z

istioctl/cmd/istioctl/main.go

+			Usage:    gf.Usage + ". Like -v flag. ex: --vklog=9",
+		}))
+
+	klog.SetLogger(nil)


Can we at least remove klog.SetLogger(nil) so it uses our logging system?

mandarjog · 2021-06-10T06:49:20Z

@howardjohn I removed a bunch of code and now setting --vklog will correctly adjust the loglevel for klog so the user does not need to provide additional --log_output_level=klog:debug.
I will move all this into istio.io/pkg soon.

mandarjog · 2021-06-10T06:50:48Z

@esnible @therealmitchconnors for UX.

mandarjog · 2021-06-10T13:30:09Z

/test integ-security-multicluster-tests_istio

mandarjog requested review from richardwxn and ostromart May 26, 2021 15:51

mandarjog requested review from a team as code owners May 26, 2021 15:51

istio-testing added the do-not-merge/work-in-progress Block merging of a PR because it isn't ready yet. label May 26, 2021

google-cla bot added the cla: yes Set by the Google CLA bot to indicate the author of a PR has signed the Google CLA. label May 26, 2021

istio-testing added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label May 26, 2021

howardjohn reviewed May 26, 2021

View reviewed changes

mandarjog requested review from costinm, linsun and nmittler as code owners June 8, 2021 07:01

istio-testing added the needs-rebase Indicates a PR needs to be rebased before being merged label Jun 8, 2021

mandarjog added 6 commits June 8, 2021 07:06

Add one istio version in user agent

fecb5c2

all files

664c571

use version alias

cf8020d

make gen

bccd999

wip2

20731e8

Add v log and confirm with DebugWrap

f64f24e

mandarjog force-pushed the istio_user_agent branch from 2287196 to f64f24e Compare June 8, 2021 07:17

istio-testing removed the needs-rebase Indicates a PR needs to be rebased before being merged label Jun 8, 2021

mandarjog changed the title ~~WIP: Istio user agent~~ Add Istio user agent to kube-client used in istioctl and other binaries Jun 8, 2021

istio-testing removed the do-not-merge/work-in-progress Block merging of a PR because it isn't ready yet. label Jun 8, 2021

mandarjog added 4 commits June 8, 2021 22:29

reduce flag

30130e8

fix gen

326d93b

WIP: check if logging makes tests fail

98bc572

fix gen

41299a5

mandarjog force-pushed the istio_user_agent branch from 63e41b3 to 41299a5 Compare June 9, 2021 04:22

remove debug

debef00

mandarjog added 2 commits June 9, 2021 06:18

remove timeout, it does not work with POST?

71f971f

use timeout only for bug-report as before

ae36ffa

howardjohn reviewed Jun 9, 2021

View reviewed changes

howardjohn approved these changes Jun 10, 2021

View reviewed changes

mandarjog added 2 commits June 10, 2021 06:39

align with istio.io/pkg/log

58e80e9

add release note

14db30d

mandarjog requested a review from a team as a code owner June 10, 2021 06:46

howardjohn approved these changes Jun 10, 2021

View reviewed changes

esnible approved these changes Jun 10, 2021

View reviewed changes

istio-testing merged commit 177cd70 into istio:master Jun 10, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Istio user agent to kube-client used in istioctl and other binaries #33120

Add Istio user agent to kube-client used in istioctl and other binaries #33120

mandarjog commented May 26, 2021 •

edited

Loading

howardjohn left a comment

howardjohn May 26, 2021

howardjohn May 26, 2021

mandarjog commented Jun 9, 2021

howardjohn left a comment

howardjohn Jun 9, 2021

howardjohn Jun 9, 2021

mandarjog Jun 9, 2021

howardjohn Jun 10, 2021

howardjohn Jun 10, 2021

mandarjog commented Jun 10, 2021 •

edited

Loading

mandarjog commented Jun 10, 2021

mandarjog commented Jun 10, 2021

Add Istio user agent to kube-client used in istioctl and other binaries #33120

Add Istio user agent to kube-client used in istioctl and other binaries #33120

Conversation

mandarjog commented May 26, 2021 • edited Loading

howardjohn left a comment

Choose a reason for hiding this comment

howardjohn May 26, 2021

Choose a reason for hiding this comment

howardjohn May 26, 2021

Choose a reason for hiding this comment

mandarjog commented Jun 9, 2021

howardjohn left a comment

Choose a reason for hiding this comment

howardjohn Jun 9, 2021

Choose a reason for hiding this comment

howardjohn Jun 9, 2021

Choose a reason for hiding this comment

mandarjog Jun 9, 2021

Choose a reason for hiding this comment

howardjohn Jun 10, 2021

Choose a reason for hiding this comment

howardjohn Jun 10, 2021

Choose a reason for hiding this comment

mandarjog commented Jun 10, 2021 • edited Loading

mandarjog commented Jun 10, 2021

mandarjog commented Jun 10, 2021

mandarjog commented May 26, 2021 •

edited

Loading

mandarjog commented Jun 10, 2021 •

edited

Loading