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

fix(logClient.go):fix panic interface conversion: interface {} is nil not string #292

Merged
merged 1 commit into from
May 22, 2023

Conversation

xiao-jay
Copy link
Contributor

fix #290

First

wrong code: https://github.com/kubearmor/kubearmor-client/blob/main/log/logClient.go#L304

func WatchTelemetryHelper(arr []byte, t string, o Options) {
	var res map[string]interface{}
	err := json.Unmarshal(arr, &res)
	if err != nil {
		return
	}

	// Filter Telemetry based on provided options
	if len(o.Selector) != 0 {
304		val := selectLabels(o, res["Labels"].(string))
		if val != nil {
			return
		}
	}

	if o.Namespace != "" {
		ns, ok := res["NamespaceName"].(string)
		if !ok {
			return
		}
		match := regexMatcher(CNamespace, ns)
		if !match {
			return
		}
	}

some time len(o.Selector) != 0 but res["Labels"] is nil, will lead to panic.

Second

if o.Selector match one label should select it ? so i use labels := strings.Split(res["Labels"].(string), ",") to split multiple labels.

…, not string

Signed-off-by: 晓杰 <2561589453@qq.com>
Copy link
Member

@Shreyas220 Shreyas220 left a comment

Choose a reason for hiding this comment

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

This PR solves two thing

1 Solves the panic issue

Adding a check res["Labels"] != nil before accessing res["Labels"].(string)

2 Multiple label

Before this if we had multiple labels , we would return nil,
by adding this we are separate the label and check

cc: @Ankurk99

LGTM 👍

@daemon1024 daemon1024 merged commit f39985f into kubearmor:main May 22, 2023
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 this pull request may close these issues.

bug(karmor logs): karmor logs are getting panicked when we use label function
4 participants