Skip to content

Commit

Permalink
Enable authz check for specific namespace. (#1592)
Browse files Browse the repository at this point in the history 
* Enable authz check for specific namespace.

* Switch to namespaced auth for tiller-proxy also.

* Update comment

* Update to assume empty namespace (or _all) means cluster-wide access.

* Update fetchCharts to use namespace from UI (#1593)

* Update fetchCharts to use namespace from UI

* Use the correct resource name

* Use namespace from the chart when rendering responses. (#1596)

* Use namespace from the chart when rendering responses.

* Update tiller-proxy to avoid sending values in request contexts.

* Remove extra dev args
  • Loading branch information
@absoludity
absoludity authored Mar 23, 2020
1 parent abff101 commit 3a2acab
Show file tree
Hide file tree
Showing 17 changed files with 245 additions and 199 deletions.
24 changes: 13 additions & 11 deletions cmd/assetsvc/handler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ func uniqChartList(charts []*models.Chart) []*models.Chart {

func getPaginatedChartList(namespace, repo string, pageNumber, pageSize int, showDuplicates bool) (apiListResponse, interface{}, error) {
charts, totalPages, err := manager.getPaginatedChartList(namespace, repo, pageNumber, pageSize, showDuplicates)
return newChartListResponse(namespace, charts), meta{totalPages}, err
return newChartListResponse(charts), meta{totalPages}, err
}

// listCharts returns a list of charts based on filter params
Expand All @@ -140,7 +140,7 @@ func getChart(w http.ResponseWriter, req *http.Request, params Params) {
return
}

cr := newChartResponse(params["namespace"], &chart)
cr := newChartResponse(&chart)
response.NewDataResponse(cr).Write(w)
}

Expand All @@ -154,7 +154,7 @@ func listChartVersions(w http.ResponseWriter, req *http.Request, params Params)
return
}

cvl := newChartVersionListResponse(params["namespace"], &chart)
cvl := newChartVersionListResponse(&chart)
response.NewDataResponse(cvl).Write(w)
}

Expand All @@ -168,7 +168,7 @@ func getChartVersion(w http.ResponseWriter, req *http.Request, params Params) {
return
}

cvr := newChartVersionResponse(params["namespace"], &chart, chart.ChartVersions[0])
cvr := newChartVersionResponse(&chart, chart.ChartVersions[0])
response.NewDataResponse(cvr).Write(w)
}

Expand Down Expand Up @@ -255,12 +255,13 @@ func listChartsWithFilters(w http.ResponseWriter, req *http.Request, params Para
if !showDuplicates(req) {
chartResponse = uniqChartList(charts)
}
cl := newChartListResponse(params["namespace"], chartResponse)
cl := newChartListResponse(chartResponse)
response.NewDataResponse(cl).Write(w)
}

func newChartResponse(namespace string, c *models.Chart) *apiResponse {
func newChartResponse(c *models.Chart) *apiResponse {
latestCV := c.ChartVersions[0]
namespace := c.Repo.Namespace
chartPath := fmt.Sprintf("%s/ns/%s/charts/", pathPrefix, namespace)
return &apiResponse{
Type: "chart",
Expand All @@ -285,10 +286,10 @@ func blankRawIconAndChartVersions(c models.Chart) models.Chart {
return c
}

func newChartListResponse(namespace string, charts []*models.Chart) apiListResponse {
func newChartListResponse(charts []*models.Chart) apiListResponse {
cl := apiListResponse{}
for _, c := range charts {
cl = append(cl, newChartResponse(namespace, c))
cl = append(cl, newChartResponse(c))
}
return cl
}
Expand All @@ -310,7 +311,8 @@ func chartAttributes(namespace string, c models.Chart) models.Chart {
return c
}

func newChartVersionResponse(namespace string, c *models.Chart, cv models.ChartVersion) *apiResponse {
func newChartVersionResponse(c *models.Chart, cv models.ChartVersion) *apiResponse {
namespace := c.Repo.Namespace
chartPath := fmt.Sprintf("%s/ns/%s/charts/%s", pathPrefix, namespace, c.ID)
return &apiResponse{
Type: "chartVersion",
Expand All @@ -326,10 +328,10 @@ func newChartVersionResponse(namespace string, c *models.Chart, cv models.ChartV
}
}

func newChartVersionListResponse(namespace string, c *models.Chart) apiListResponse {
func newChartVersionListResponse(c *models.Chart) apiListResponse {
var cvl apiListResponse
for _, cv := range c.ChartVersions {
cvl = append(cvl, newChartVersionResponse(namespace, c, cv))
cvl = append(cvl, newChartVersionResponse(c, cv))
}

return cvl
Expand Down
85 changes: 44 additions & 41 deletions cmd/assetsvc/handler_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,11 @@ const (
testChartValues = "image:\n registry: docker.io\n repository: my-repo/my-chart\n tag: 0.1.0"
testChartSchema = `{"properties": {"type": "object"}}`
namespace = "kubeapps-namespace"
testRepoName = "my-repo"
)

var testRepo *models.Repo = &models.Repo{Name: testRepoName, Namespace: namespace}

func iconBytes() []byte {
var b bytes.Buffer
img := imaging.New(1, 1, color.White)
Expand Down Expand Up @@ -121,18 +124,18 @@ func Test_newChartResponse(t *testing.T) {
chart models.Chart
}{
{"chart has only one version", models.Chart{
ID: "my-repo/my-chart", ChartVersions: []models.ChartVersion{{Version: "1.2.3"}}},
Repo: testRepo, ID: "my-repo/my-chart", ChartVersions: []models.ChartVersion{{Version: "1.2.3"}}},
},
{"chart has many versions", models.Chart{
ID: "my-repo/my-chart", ChartVersions: []models.ChartVersion{{Version: "0.1.2"}, {Version: "0.1.0"}},
Repo: testRepo, ID: "my-repo/my-chart", ChartVersions: []models.ChartVersion{{Version: "0.1.2"}, {Version: "0.1.0"}},
}},
{"raw_icon is never sent down the wire", models.Chart{
ID: "my-repo/my-chart", ChartVersions: []models.ChartVersion{{Version: "1.2.3"}}, RawIcon: iconBytes(), IconContentType: "image/svg",
Repo: testRepo, ID: "my-repo/my-chart", ChartVersions: []models.ChartVersion{{Version: "1.2.3"}}, RawIcon: iconBytes(), IconContentType: "image/svg",
}},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
cResponse := newChartResponse(namespace, &tt.chart)
cResponse := newChartResponse(&tt.chart)
assert.Equal(t, cResponse.Type, "chart", "response type is chart")
assert.Equal(t, cResponse.ID, tt.chart.ID, "chart ID should be the same")
assert.Equal(t, cResponse.Relationships["latestChartVersion"].Data.(models.ChartVersion).Version, tt.chart.ChartVersions[0].Version, "latestChartVersion should match version at index 0")
Expand All @@ -151,22 +154,22 @@ func Test_newChartListResponse(t *testing.T) {
}{
{"no charts", []*models.Chart{}, []*models.Chart{}},
{"has one chart", []*models.Chart{
{ID: "my-repo/my-chart", ChartVersions: []models.ChartVersion{{Version: "0.0.1", Digest: "123"}}},
{Repo: testRepo, ID: "my-repo/my-chart", ChartVersions: []models.ChartVersion{{Version: "0.0.1", Digest: "123"}}},
}, []*models.Chart{
{ID: "my-repo/my-chart", ChartVersions: []models.ChartVersion{{Version: "0.0.1", Digest: "123"}}},
{Repo: testRepo, ID: "my-repo/my-chart", ChartVersions: []models.ChartVersion{{Version: "0.0.1", Digest: "123"}}},
}},
{"has two charts", []*models.Chart{
{ID: "my-repo/my-chart", ChartVersions: []models.ChartVersion{{Version: "0.0.1", Digest: "123"}}},
{ID: "stable/wordpress", ChartVersions: []models.ChartVersion{{Version: "1.2.3", Digest: "1234"}, {Version: "1.2.2", Digest: "12345"}}},
{Repo: testRepo, ID: "my-repo/my-chart", ChartVersions: []models.ChartVersion{{Version: "0.0.1", Digest: "123"}}},
{Repo: testRepo, ID: "stable/wordpress", ChartVersions: []models.ChartVersion{{Version: "1.2.3", Digest: "1234"}, {Version: "1.2.2", Digest: "12345"}}},
}, []*models.Chart{
{ID: "my-repo/my-chart", ChartVersions: []models.ChartVersion{{Version: "0.0.1", Digest: "123"}}},
{ID: "stable/wordpress", ChartVersions: []models.ChartVersion{{Version: "1.2.3", Digest: "1234"}, {Version: "1.2.2", Digest: "12345"}}},
{Repo: testRepo, ID: "my-repo/my-chart", ChartVersions: []models.ChartVersion{{Version: "0.0.1", Digest: "123"}}},
{Repo: testRepo, ID: "stable/wordpress", ChartVersions: []models.ChartVersion{{Version: "1.2.3", Digest: "1234"}, {Version: "1.2.2", Digest: "12345"}}},
}},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
clResponse := newChartListResponse(namespace, tt.input)
clResponse := newChartListResponse(tt.input)
assert.Equal(t, len(clResponse), len(tt.result), "number of charts in response should be the same")
for i := range tt.result {
assert.Equal(t, clResponse[i].Type, "chart", "response type is chart")
Expand All @@ -187,13 +190,13 @@ func Test_newChartVersionResponse(t *testing.T) {
{
name: "my-chart",
chart: models.Chart{
ID: "my-repo/my-chart", ChartVersions: []models.ChartVersion{{Version: "0.1.0"}, {Version: "0.2.3"}},
Repo: testRepo, ID: "my-repo/my-chart", ChartVersions: []models.ChartVersion{{Version: "0.1.0"}, {Version: "0.2.3"}},
},
},
{
name: "RawIcon is never sent down the wire",
chart: models.Chart{
ID: "my-repo/my-chart", ChartVersions: []models.ChartVersion{{Version: "1.2.3"}}, RawIcon: iconBytes(), IconContentType: "image/svg",
Repo: testRepo, ID: "my-repo/my-chart", ChartVersions: []models.ChartVersion{{Version: "1.2.3"}}, RawIcon: iconBytes(), IconContentType: "image/svg",
},
expectedIcon: "/v1/ns/" + namespace + "/assets/my-repo/my-chart/logo",
},
Expand All @@ -202,7 +205,7 @@ func Test_newChartVersionResponse(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
for i := range tt.chart.ChartVersions {
cvResponse := newChartVersionResponse(namespace, &tt.chart, tt.chart.ChartVersions[i])
cvResponse := newChartVersionResponse(&tt.chart, tt.chart.ChartVersions[i])
assert.Equal(t, cvResponse.Type, "chartVersion", "response type is chartVersion")
assert.Equal(t, cvResponse.ID, tt.chart.ID+"-"+tt.chart.ChartVersions[i].Version, "reponse id should have chart version suffix")
assert.Equal(t, cvResponse.Links.(interface{}).(selfLink).Self, pathPrefix+"/ns/"+namespace+"/charts/"+tt.chart.ID+"/versions/"+tt.chart.ChartVersions[i].Version, "self link should be the same")
Expand All @@ -225,19 +228,19 @@ func Test_newChartVersionListResponse(t *testing.T) {
chart models.Chart
}{
{"chart has no versions", models.Chart{
ID: "my-repo/my-chart", ChartVersions: []models.ChartVersion{},
Repo: testRepo, ID: "my-repo/my-chart", ChartVersions: []models.ChartVersion{},
}},
{"chart has one version", models.Chart{
ID: "my-repo/my-chart", ChartVersions: []models.ChartVersion{{Version: "0.0.1"}},
Repo: testRepo, ID: "my-repo/my-chart", ChartVersions: []models.ChartVersion{{Version: "0.0.1"}},
}},
{"chart has many versions", models.Chart{
ID: "my-repo/my-chart", ChartVersions: []models.ChartVersion{{Version: "0.0.1"}, {Version: "0.0.2"}},
Repo: testRepo, ID: "my-repo/my-chart", ChartVersions: []models.ChartVersion{{Version: "0.0.1"}, {Version: "0.0.2"}},
}},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
cvListResponse := newChartVersionListResponse(namespace, &tt.chart)
cvListResponse := newChartVersionListResponse(&tt.chart)
assert.Equal(t, len(cvListResponse), len(tt.chart.ChartVersions), "number of chart versions in response should be the same")
for i := range tt.chart.ChartVersions {
assert.Equal(t, cvListResponse[i].Type, "chartVersion", "response type is chartVersion")
Expand All @@ -258,18 +261,18 @@ func Test_listCharts(t *testing.T) {
}{
{"no charts", "", []*models.Chart{}, meta{1}},
{"one chart", "", []*models.Chart{
{ID: "my-repo/my-chart", ChartVersions: []models.ChartVersion{{Version: "0.0.1", Digest: "123"}}},
{Repo: testRepo, ID: "my-repo/my-chart", ChartVersions: []models.ChartVersion{{Version: "0.0.1", Digest: "123"}}},
}, meta{1}},
{"two charts", "", []*models.Chart{
{ID: "my-repo/my-chart", ChartVersions: []models.ChartVersion{{Version: "0.0.1", Digest: "123"}}},
{ID: "stable/dokuwiki", ChartVersions: []models.ChartVersion{{Version: "1.2.3", Digest: "1234"}, {Version: "1.2.2", Digest: "12345"}}},
{Repo: testRepo, ID: "my-repo/my-chart", ChartVersions: []models.ChartVersion{{Version: "0.0.1", Digest: "123"}}},
{Repo: testRepo, ID: "stable/dokuwiki", ChartVersions: []models.ChartVersion{{Version: "1.2.3", Digest: "1234"}, {Version: "1.2.2", Digest: "12345"}}},
}, meta{1}},
// Pagination tests
{"four charts with pagination", "?size=2", []*models.Chart{
{ID: "my-repo/my-chart", ChartVersions: []models.ChartVersion{{Version: "0.0.1", Digest: "123"}}},
{ID: "stable/dokuwiki", ChartVersions: []models.ChartVersion{{Version: "1.2.3", Digest: "1234"}}},
{ID: "stable/drupal", ChartVersions: []models.ChartVersion{{Version: "1.2.3", Digest: "12345"}}},
{ID: "stable/wordpress", ChartVersions: []models.ChartVersion{{Version: "1.2.3", Digest: "123456"}}},
{Repo: testRepo, ID: "my-repo/my-chart", ChartVersions: []models.ChartVersion{{Version: "0.0.1", Digest: "123"}}},
{Repo: testRepo, ID: "stable/dokuwiki", ChartVersions: []models.ChartVersion{{Version: "1.2.3", Digest: "1234"}}},
{Repo: testRepo, ID: "stable/drupal", ChartVersions: []models.ChartVersion{{Version: "1.2.3", Digest: "12345"}}},
{Repo: testRepo, ID: "stable/wordpress", ChartVersions: []models.ChartVersion{{Version: "1.2.3", Digest: "123456"}}},
}, meta{2}},
}

Expand Down Expand Up @@ -321,17 +324,17 @@ func Test_listRepoCharts(t *testing.T) {
}{
{"repo has no charts", "my-repo", "", []*models.Chart{}, meta{1}},
{"repo has one chart", "my-repo", "", []*models.Chart{
{ID: "my-repo/my-chart", ChartVersions: []models.ChartVersion{{Version: "0.0.1", Digest: "123"}}},
{Repo: testRepo, ID: "my-repo/my-chart", ChartVersions: []models.ChartVersion{{Version: "0.0.1", Digest: "123"}}},
}, meta{1}},
{"repo has many charts", "my-repo", "", []*models.Chart{
{ID: "my-repo/my-chart", ChartVersions: []models.ChartVersion{{Version: "0.0.1", Digest: "123"}}},
{ID: "my-repo/dokuwiki", ChartVersions: []models.ChartVersion{{Version: "1.2.3", Digest: "1234"}, {Version: "1.2.2", Digest: "12345"}}},
{Repo: testRepo, ID: "my-repo/my-chart", ChartVersions: []models.ChartVersion{{Version: "0.0.1", Digest: "123"}}},
{Repo: testRepo, ID: "my-repo/dokuwiki", ChartVersions: []models.ChartVersion{{Version: "1.2.3", Digest: "1234"}, {Version: "1.2.2", Digest: "12345"}}},
}, meta{1}},
{"repo has many charts with pagination", "my-repo", "?size=2", []*models.Chart{
{ID: "my-repo/my-chart", ChartVersions: []models.ChartVersion{{Version: "0.0.1", Digest: "123"}}},
{ID: "stable/dokuwiki", ChartVersions: []models.ChartVersion{{Version: "1.2.3", Digest: "1234"}}},
{ID: "stable/drupal", ChartVersions: []models.ChartVersion{{Version: "1.2.3", Digest: "12345"}}},
{ID: "stable/wordpress", ChartVersions: []models.ChartVersion{{Version: "1.2.3", Digest: "123456"}}},
{Repo: testRepo, ID: "my-repo/my-chart", ChartVersions: []models.ChartVersion{{Version: "0.0.1", Digest: "123"}}},
{Repo: testRepo, ID: "stable/dokuwiki", ChartVersions: []models.ChartVersion{{Version: "1.2.3", Digest: "1234"}}},
{Repo: testRepo, ID: "stable/drupal", ChartVersions: []models.ChartVersion{{Version: "1.2.3", Digest: "12345"}}},
{Repo: testRepo, ID: "stable/wordpress", ChartVersions: []models.ChartVersion{{Version: "1.2.3", Digest: "123456"}}},
}, meta{2}},
}

Expand Down Expand Up @@ -384,19 +387,19 @@ func Test_getChart(t *testing.T) {
{
"chart does not exist",
errors.New("return an error when checking if chart exists"),
models.Chart{ID: "my-repo/my-chart"},
models.Chart{Repo: testRepo, ID: "my-repo/my-chart"},
http.StatusNotFound,
},
{
"chart exists",
nil,
models.Chart{ID: "my-repo/my-chart", ChartVersions: []models.ChartVersion{{Version: "0.1.0"}}},
models.Chart{Repo: testRepo, ID: "my-repo/my-chart", ChartVersions: []models.ChartVersion{{Version: "0.1.0"}}},
http.StatusOK,
},
{
"chart has multiple versions",
nil,
models.Chart{ID: "my-repo/my-chart", ChartVersions: []models.ChartVersion{{Version: "0.1.0"}, {Version: "0.0.1"}}},
models.Chart{Repo: testRepo, ID: "my-repo/my-chart", ChartVersions: []models.ChartVersion{{Version: "0.1.0"}, {Version: "0.0.1"}}},
http.StatusOK,
},
}
Expand Down Expand Up @@ -449,19 +452,19 @@ func Test_listChartVersions(t *testing.T) {
{
"chart does not exist",
errors.New("return an error when checking if chart exists"),
models.Chart{ID: "my-repo/my-chart"},
models.Chart{Repo: testRepo, ID: "my-repo/my-chart"},
http.StatusNotFound,
},
{
"chart exists",
nil,
models.Chart{ID: "my-repo/my-chart", ChartVersions: []models.ChartVersion{{Version: "0.1.0"}}},
models.Chart{Repo: testRepo, ID: "my-repo/my-chart", ChartVersions: []models.ChartVersion{{Version: "0.1.0"}}},
http.StatusOK,
},
{
"chart has multiple versions",
nil,
models.Chart{ID: "my-repo/my-chart", ChartVersions: []models.ChartVersion{{Version: "0.1.0"}, {Version: "0.0.1"}}},
models.Chart{Repo: testRepo, ID: "my-repo/my-chart", ChartVersions: []models.ChartVersion{{Version: "0.1.0"}, {Version: "0.0.1"}}},
http.StatusOK,
},
}
Expand Down Expand Up @@ -515,19 +518,19 @@ func Test_getChartVersion(t *testing.T) {
{
"chart does not exist",
errors.New("return an error when checking if chart exists"),
models.Chart{ID: "my-repo/my-chart", ChartVersions: []models.ChartVersion{{Version: "0.1.0"}}},
models.Chart{Repo: testRepo, ID: "my-repo/my-chart", ChartVersions: []models.ChartVersion{{Version: "0.1.0"}}},
http.StatusNotFound,
},
{
"chart exists",
nil,
models.Chart{ID: "my-repo/my-chart", ChartVersions: []models.ChartVersion{{Version: "0.1.0"}}},
models.Chart{Repo: testRepo, ID: "my-repo/my-chart", ChartVersions: []models.ChartVersion{{Version: "0.1.0"}}},
http.StatusOK,
},
{
"chart has multiple versions",
nil,
models.Chart{ID: "my-repo/my-chart", ChartVersions: []models.ChartVersion{{Version: "0.1.0"}, {Version: "0.0.1"}}},
models.Chart{Repo: testRepo, ID: "my-repo/my-chart", ChartVersions: []models.ChartVersion{{Version: "0.1.0"}, {Version: "0.0.1"}}},
http.StatusOK,
},
}
Expand Down
Loading

0 comments on commit 3a2acab

Please sign in to comment.