istio / old_pilot_repo

Deprecated home of Istio's Pilot, now in istio/istio's pilot dir
Apache License 2.0
137 stars 91 forks source link

fix nil pointer dereference when current kubectl context is empty #1667

Closed ayj closed 6 years ago

ayj commented 6 years ago

The following panic occurs if kubectl's current-context does not point to a valid configuration context.

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x30 pc=0x11f2018]

goroutine 1 [running]:
main.getDefaultNamespace(0xc4202cc280, 0x12, 0x0, 0x0)
cmd/istioctl/main.go:700 +0xb8
main.glob..func4(0x1c35de0, 0x1c61be8, 0x0, 0x0)
cmd/istioctl/main.go:86 +0x39
github.com/spf13/cobra.(*Command).execute(0x1c35de0, 0x1c61be8, 0x0, 0x0, 0x1c35de0, 0x1c61be8)
external/com_github_spf13_cobra/command.go:637 +0x549
github.com/spf13/cobra.(*Command).ExecuteC(0x1c349a0, 0x41172d, 0xc4200002a8, 0x0)
external/com_github_spf13_cobra/command.go:729 +0x339
github.com/spf13/cobra.(*Command).Execute(0x1c349a0, 0xc4204f7f78, 0x434056)
external/com_github_spf13_cobra/command.go:688 +0x2b
main.main()
cmd/istioctl/main.go:531 +0xc0

fixes https://github.com/istio/issues/issues/105

NONE
istio-testing commented 6 years ago

@ayj: Adding do-not-merge/release-note-label-needed because the release note process has not been followed.

One of the following labels is required "release-note", "release-note-action-required", or "release-note-none". Please see: https://github.com/kubernetes/community/blob/master/contributors/devel/pull-requests.md#write-release-notes-if-needed. Instructions for interacting with me using PR comments are available [here](https://github.com/kubernetes/community/blob/master/contributors/devel/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
istio-testing commented 6 years ago

@ayj: Adding do-not-merge/release-note-label-needed because the release note process has not been followed.

One of the following labels is required "release-note", "release-note-action-required", or "release-note-none". Please see: https://github.com/kubernetes/community/blob/master/contributors/devel/pull-requests.md#write-release-notes-if-needed. Instructions for interacting with me using PR comments are available [here](https://github.com/kubernetes/community/blob/master/contributors/devel/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
rkpagadala commented 6 years ago

/lgtm

istio-testing commented 6 years ago

@rkpagadala: changing LGTM is restricted to assignees, and assigning you to the PR failed.

In response to [this](https://github.com/istio/pilot/pull/1667#issuecomment-341180736): >/lgtm Instructions for interacting with me using PR comments are available [here](https://github.com/kubernetes/community/blob/master/contributors/devel/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
rkpagadala commented 6 years ago

/approve

geeknoid commented 6 years ago

This repo is no longer accepting PRs. Please resubmit this change to the istio/istio repo.

Thanks.

ldemailly commented 6 years ago

Let's not close releaze-0.2 bug fixes

ldemailly commented 6 years ago

Conflict?

jmuk commented 6 years ago

looks conflicting my recent fix (https://github.com/istio/old_pilot_repo/pull/1638), be careful to resolve without breaking my one.

ldemailly commented 6 years ago

@ayj ping, is this needed for 0.2 ?

codecov[bot] commented 6 years ago

Codecov Report

Merging #1667 into release-0.2 will not change coverage. The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##           release-0.2    #1667   +/-   ##
============================================
  Coverage        82.69%   82.69%           
============================================
  Files               52       52           
  Lines             6426     6426           
============================================
  Hits              5314     5314           
  Misses             909      909           
  Partials           203      203

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 998e0e0...34b77a1. Read the comment docs.

ldemailly commented 6 years ago

so we just missed 0.2.11 / what's the impact ?

ayj commented 6 years ago

Users need to specify a valid kubeconfig with --kubeconfig or have a valid default config discoverable through KUBECONFIG

ldemailly commented 6 years ago

k not too bad, connectivity to the cluster is needed for kube-inject anyway for instance