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.

Sign up for GitHub

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

Remove namespace usage #782

Merged
merged 4 commits into from
Nov 6, 2018
Merged

Remove namespace usage #782

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c115b63
Remove namespace usage
migmartri Nov 1, 2018
f6de93b
Revert "Remove namespace usage"
migmartri Nov 5, 2018
cd6ddf9
Check namespace
migmartri Nov 6, 2018
7535a81
Merge branch 'master' into 765-remove-namespace
migmartri Nov 6, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 7 additions & 8 deletions cmd/tiller-proxy/internal/handler/handler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,12 @@ import (

"github.com/gorilla/mux"
"github.com/kubeapps/common/response"
log "github.com/sirupsen/logrus"
"github.com/urfave/negroni"
"k8s.io/helm/pkg/proto/hapi/chart"

"github.com/kubeapps/kubeapps/pkg/auth"
chartUtils "github.com/kubeapps/kubeapps/pkg/chart"
proxy "github.com/kubeapps/kubeapps/pkg/proxy"
log "github.com/sirupsen/logrus"
"github.com/urfave/negroni"
"k8s.io/helm/pkg/proto/hapi/chart"
)

// Context key type for request contexts
Expand Down Expand Up @@ -221,7 +220,7 @@ func (h *TillerProxy) UpgradeRelease(w http.ResponseWriter, req *http.Request, p
return
}
}
rel, err := h.ProxyClient.UpdateRelease(params["releaseName"], params["namespace"], chartDetails.Values, ch)
rel, err := h.ProxyClient.UpdateRelease(params["releaseName"], chartDetails.Values, ch)
if err != nil {
response.NewErrorResponse(errorCodeWithDefault(err, http.StatusUnprocessableEntity), err.Error()).Write(w)
return
Expand Down Expand Up @@ -253,7 +252,7 @@ func (h *TillerProxy) ListReleases(w http.ResponseWriter, req *http.Request, par

// GetRelease returns the release info
func (h *TillerProxy) GetRelease(w http.ResponseWriter, req *http.Request, params Params) {
rel, err := h.ProxyClient.GetRelease(params["releaseName"], params["namespace"])
rel, err := h.ProxyClient.GetRelease(params["releaseName"])
if err != nil {
response.NewErrorResponse(errorCode(err), err.Error()).Write(w)
return
Expand Down Expand Up @@ -281,7 +280,7 @@ func (h *TillerProxy) GetRelease(w http.ResponseWriter, req *http.Request, param
// DeleteRelease removes a release from a namespace
func (h *TillerProxy) DeleteRelease(w http.ResponseWriter, req *http.Request, params Params) {
if !h.DisableAuth {
rel, err := h.ProxyClient.GetRelease(params["releaseName"], params["namespace"])
rel, err := h.ProxyClient.GetRelease(params["releaseName"])
if err != nil {
response.NewErrorResponse(errorCode(err), err.Error()).Write(w)
return
Expand All @@ -306,7 +305,7 @@ func (h *TillerProxy) DeleteRelease(w http.ResponseWriter, req *http.Request, pa
if req.URL.Query().Get("purge") == "1" || req.URL.Query().Get("purge") == "true" {
purge = true
}
err := h.ProxyClient.DeleteRelease(params["releaseName"], params["namespace"], purge)
err := h.ProxyClient.DeleteRelease(params["releaseName"], purge)
if err != nil {
response.NewErrorResponse(errorCode(err), err.Error()).Write(w)
return
Expand Down
9 changes: 5 additions & 4 deletions cmd/tiller-proxy/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ import (

"github.com/gorilla/mux"
"github.com/heptiolabs/healthcheck"
"github.com/kubeapps/kubeapps/cmd/tiller-proxy/internal/handler"
chartUtils "github.com/kubeapps/kubeapps/pkg/chart"
tillerProxy "github.com/kubeapps/kubeapps/pkg/proxy"
log "github.com/sirupsen/logrus"
"github.com/spf13/pflag"
"github.com/urfave/negroni"
Expand All @@ -33,10 +36,6 @@ import (
"k8s.io/helm/pkg/helm"
"k8s.io/helm/pkg/helm/environment"
"k8s.io/helm/pkg/tlsutil"

"github.com/kubeapps/kubeapps/cmd/tiller-proxy/internal/handler"
chartUtils "github.com/kubeapps/kubeapps/pkg/chart"
tillerProxy "github.com/kubeapps/kubeapps/pkg/proxy"
)

const (
Expand Down Expand Up @@ -144,6 +143,8 @@ func main() {
ProxyClient: proxy,
}
// Routes
// TODO Some of these routes do not require namespace anymore (i.e getRelease)
Copy link
Contributor

Choose a reason for hiding this comment

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

actually, all the routes do require the namespace, because we render the manifest with ResolveManifest and go through each kind to ensure a user can get, create, update or delete the kind in the namespace.

Copy link
Contributor

Choose a reason for hiding this comment

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

oh, true, I forgot about that.

// it has been kept this way for compatibility reasons. Change.
apiv1 := r.PathPrefix("/v1").Subrouter()
apiv1.Methods("GET").Path("/releases").Handler(negroni.New(
authGate,
Expand Down
6 changes: 3 additions & 3 deletions pkg/proxy/fake/proxy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ func (f *FakeProxy) CreateRelease(name, namespace, values string, ch *chart.Char
return &r, nil
}

func (f *FakeProxy) UpdateRelease(name, namespace string, values string, ch *chart.Chart) (*release.Release, error) {
func (f *FakeProxy) UpdateRelease(name string, values string, ch *chart.Chart) (*release.Release, error) {
for _, r := range f.Releases {
if r.Name == name {
return &r, nil
Expand All @@ -82,7 +82,7 @@ func (f *FakeProxy) UpdateRelease(name, namespace string, values string, ch *cha
return nil, fmt.Errorf("Release %s not found", name)
}

func (f *FakeProxy) GetRelease(name, namespace string) (*release.Release, error) {
func (f *FakeProxy) GetRelease(name string) (*release.Release, error) {
for _, r := range f.Releases {
if r.Name == name {
return &r, nil
Expand All @@ -91,7 +91,7 @@ func (f *FakeProxy) GetRelease(name, namespace string) (*release.Release, error)
return nil, fmt.Errorf("Release %s not found", name)
}

func (f *FakeProxy) DeleteRelease(name, namespace string, purge bool) error {
func (f *FakeProxy) DeleteRelease(name string, purge bool) error {
for i, r := range f.Releases {
if r.Name == name {
if purge {
Expand Down
24 changes: 11 additions & 13 deletions pkg/proxy/proxy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,9 +78,7 @@ type AppOverview struct {
Status string `json:"status"`
}

// TODO: Rename get for getRelease
// TODO: Remove namespace since release name is unique
func (p *Proxy) get(name, namespace string) (*release.Release, error) {
func (p *Proxy) getRelease(name string) (*release.Release, error) {
release, err := p.helmClient.ReleaseContent(name)

if err != nil {
Expand Down Expand Up @@ -236,11 +234,11 @@ func (p *Proxy) CreateRelease(name, namespace, values string, ch *chart.Chart) (
}

// UpdateRelease upgrades a tiller release
func (p *Proxy) UpdateRelease(name, namespace string, values string, ch *chart.Chart) (*release.Release, error) {
func (p *Proxy) UpdateRelease(name, values string, ch *chart.Chart) (*release.Release, error) {
lock(name)
defer unlock(name)
// Check if the release already exists
_, err := p.get(name, namespace)
_, err := p.getRelease(name)
if err != nil {
return nil, err
}
Expand All @@ -258,18 +256,18 @@ func (p *Proxy) UpdateRelease(name, namespace string, values string, ch *chart.C
}

// GetRelease returns the info of a release
func (p *Proxy) GetRelease(name, namespace string) (*release.Release, error) {
func (p *Proxy) GetRelease(name string) (*release.Release, error) {
lock(name)
defer unlock(name)
return p.get(name, namespace)
return p.getRelease(name)
}

// DeleteRelease deletes a release
func (p *Proxy) DeleteRelease(name, namespace string, purge bool) error {
func (p *Proxy) DeleteRelease(name string, purge bool) error {
lock(name)
defer unlock(name)
// Validate that the release actually belongs to the namespace
_, err := p.get(name, namespace)
_, err := p.getRelease(name)
if err != nil {
return err
}
Expand Down Expand Up @@ -297,11 +295,11 @@ func prettyError(err error) error {

// TillerClient for exposed funcs
type TillerClient interface {
GetReleaseStatus(relName string) (release.Status_Code, error)
GetReleaseStatus(name string) (release.Status_Code, error)
ResolveManifest(namespace, values string, ch *chart.Chart) (string, error)
ListReleases(namespace string, releaseListLimit int, status string) ([]AppOverview, error)
CreateRelease(name, namespace, values string, ch *chart.Chart) (*release.Release, error)
UpdateRelease(name, namespace string, values string, ch *chart.Chart) (*release.Release, error)
GetRelease(name, namespace string) (*release.Release, error)
DeleteRelease(name, namespace string, purge bool) error
UpdateRelease(name string, values string, ch *chart.Chart) (*release.Release, error)
GetRelease(name string) (*release.Release, error)
DeleteRelease(name string, purge bool) error
}
32 changes: 15 additions & 17 deletions pkg/proxy/proxy_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@ func TestHelmReleaseUpdated(t *testing.T) {
app := AppOverview{rs, version, ns, "icon.png", "DEPLOYED"}
proxy := newFakeProxy([]AppOverview{app})

result, err := proxy.UpdateRelease(rs, ns, "", ch)
result, err := proxy.UpdateRelease(rs, "", ch)
if err != nil {
t.Errorf("Unexpected error %v", err)
}
Expand All @@ -272,7 +272,6 @@ func TestHelmReleaseUpdated(t *testing.T) {
}

func TestUpdateMissingHelmRelease(t *testing.T) {
ns := "myns"
rs := "foo"
chartName := "bar"
version := "v1.0.0"
Expand All @@ -285,7 +284,7 @@ func TestUpdateMissingHelmRelease(t *testing.T) {
app := AppOverview{rs2, version, ns2, "icon.png", "DEPLOYED"}
proxy := newFakeProxy([]AppOverview{app})

_, err := proxy.UpdateRelease(rs, ns, "", ch)
_, err := proxy.UpdateRelease(rs, "", ch)
if err == nil {
t.Error("Update should fail, there is not a release in the namespace specified")
}
Expand All @@ -298,21 +297,20 @@ func TestGetHelmRelease(t *testing.T) {
app1 := AppOverview{"foo", "1.0.0", "my_ns", "icon.png", "DEPLOYED"}
app2 := AppOverview{"bar", "1.0.0", "other_ns", "icon2.png", "DELETED"}
type testStruct struct {
existingApps []AppOverview
shouldFail bool
targetApp string
tartegNamespace string
expectedResult string
existingApps []AppOverview
shouldFail bool
targetApp string
expectedResult string
}
tests := []testStruct{
{[]AppOverview{app1, app2}, false, "foo", "my_ns", "foo"},
{[]AppOverview{app1, app2}, false, "foo", "", "foo"},
{[]AppOverview{app1, app2}, false, "foo", "foo"},
{[]AppOverview{app1, app2}, false, "foo", "foo"},
}
for _, test := range tests {
proxy := newFakeProxy(test.existingApps)
res, err := proxy.GetRelease(test.targetApp, test.tartegNamespace)
res, err := proxy.GetRelease(test.targetApp)
if test.shouldFail && err == nil {
t.Errorf("Get %s/%s should fail", test.tartegNamespace, test.targetApp)
t.Errorf("Get %s should fail", test.targetApp)
}
if !test.shouldFail {
if err != nil {
Expand All @@ -330,7 +328,7 @@ func TestHelmReleaseDeleted(t *testing.T) {
proxy := newFakeProxy([]AppOverview{app})

// TODO: Add a test for a non-purged release when the fake helm cli supports it
err := proxy.DeleteRelease(app.ReleaseName, app.Namespace, true)
err := proxy.DeleteRelease(app.ReleaseName, true)
if err != nil {
t.Errorf("Unexpected error %v", err)
}
Expand All @@ -347,9 +345,9 @@ func TestDeleteMissingHelmRelease(t *testing.T) {
app := AppOverview{"foo", "1.0.0", "my_ns", "icon.png", "DEPLOYED"}
proxy := newFakeProxy([]AppOverview{app})

err := proxy.DeleteRelease("not_foo", "other_ns", true)
err := proxy.DeleteRelease("not_foo", true)
if err == nil {
t.Error("Delete should fail, there is not a release in the namespace specified")
t.Error("Delete should fail, there is not a release")
}
rels, err := proxy.helmClient.ListReleases()
if err != nil {
Expand Down Expand Up @@ -390,13 +388,13 @@ func TestEnsureThreadSafety(t *testing.T) {
}
},
func() {
_, err := proxy.UpdateRelease(rs, ns, "", ch)
_, err := proxy.UpdateRelease(rs, "", ch)
if err != nil {
t.Errorf("Unexpected error %v", err)
}
},
func() {
err := proxy.DeleteRelease(rs, ns, true)
err := proxy.DeleteRelease(rs, true)
if err != nil {
t.Errorf("Unexpected error %v", err)
}
Expand Down