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

let getDefaultNamespace() returns "default" if nothing is found. #1638

Closed jmuk closed 6 years ago

jmuk commented 6 years ago

https://github.com/istio/issues/issues/110 reminds me that istioctl can set empty namespace for mixer configs, which will be delivered directly and therefore it can cause same error of "empty namespace".

When the obtained namespace happens to be empty, I think it should return the default value.

What this PR does / why we need it:

Which issue this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged): fixes #1637

Special notes for your reviewer:

Release note:

none
istio-merge-robot commented 6 years ago

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: We suggest the following additional approver: kyessenov

Assign the PR to them by writing /assign @kyessenov in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these OWNERS Files: - **[OWNERS](https://github.com/istio/pilot/blob/master/OWNERS)** You can indicate your approval by writing `/approve` in a comment You can cancel your approval by writing `/approve cancel` in a comment
jmuk commented 6 years ago

/assign @ayj

codecov[bot] commented 6 years ago

Codecov Report

Merging #1638 into master will not change coverage. The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1638   +/-   ##
=======================================
  Coverage   83.28%   83.28%           
=======================================
  Files          52       52           
  Lines        6546     6546           
=======================================
  Hits         5452     5452           
  Misses        890      890           
  Partials      204      204

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 43f959c...669f434. Read the comment docs.

jmuk commented 6 years ago

/retest

andraxylia commented 6 years ago

@jmuk could you please cherry-pick this into 0.2? Our request routing instructions no longer work.