Skip to content

Commit

Permalink
feature: support for specifying optional authorization token for HTTP…
Browse files Browse the repository at this point in the history
… requests (#247)

* feature: support for specifying bearer token for HTTP requests

* remove injection of "Bearer" string to token

* rename `ACCESS_TOKEN` to `AUTHORIZATION_HEADER`

* apprepository-controller: read authorization secretref from Spec

example:

```yaml
apiVersion: kubeapps.com/v1alpha1
kind: AppRepository
metadata:
  annotations: {}
  labels:
    created-by: kubeapps
    name: stable
  name: stable
  namespace: kubeapps
spec:
  type: helm
  url: https://kubernetes-charts.storage.googleapis.com
  auth:
    secretKeyRef:
      name: stable-repo-secret
      key: authorization-header
```

* chart-repo: adds test for authenticated HTTP client

* add-repo: validate required fields

* dashboard: adds inputbox to specify the authorization header while adding a app repository

* AccessToken -> AuthorizationHeader

* chart-repo: adds `fetchAndImportFiles` test for authenticated access

* dashboard: add namespace arg to `Secrets` api

* type: define a type for the auth field

* dashboard: add ownerReference to AppRepository while creating secret

* move `auth` to `auth.header`

* manifests: adds `POD_NAMESPACE` env variable to helm-crd controller pod

* don't use a pointer type for `AppRepositorySpec.Auth`

* address review comments

* Secret: use IOwnerReference for owner param of create
  • Loading branch information
Sameer Naik authored Apr 20, 2018
1 parent e3a1b88 commit 626bb52
Show file tree
Hide file tree
Showing 15 changed files with 308 additions and 76 deletions.
33 changes: 22 additions & 11 deletions cmd/apprepository-controller/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -434,17 +434,7 @@ func syncJobSpec(apprepo *apprepov1alpha1.AppRepository) batchv1.JobSpec {
Image: repoSyncImage,
Command: []string{"/chart-repo"},
Args: apprepoSyncJobArgs(apprepo),
Env: []corev1.EnvVar{
{
Name: "MONGO_PASSWORD",
ValueFrom: &corev1.EnvVarSource{
SecretKeyRef: &corev1.SecretKeySelector{
LocalObjectReference: corev1.LocalObjectReference{Name: "mongodb"},
Key: "mongodb-root-password",
},
},
},
},
Env: apprepoSyncJobEnvVars(apprepo),
},
},
},
Expand Down Expand Up @@ -523,6 +513,27 @@ func apprepoSyncJobArgs(apprepo *apprepov1alpha1.AppRepository) []string {
}
}

// apprepoSyncJobEnvVars returns a list of env variables for the sync container
func apprepoSyncJobEnvVars(apprepo *apprepov1alpha1.AppRepository) []corev1.EnvVar {
var envVars []corev1.EnvVar
envVars = append(envVars, corev1.EnvVar{
Name: "MONGO_PASSWORD",
ValueFrom: &corev1.EnvVarSource{
SecretKeyRef: &corev1.SecretKeySelector{
LocalObjectReference: corev1.LocalObjectReference{Name: "mongodb"},
Key: "mongodb-root-password",
},
},
})
if apprepo.Spec.Auth.Header != nil {
envVars = append(envVars, corev1.EnvVar{
Name: "AUTHORIZATION_HEADER",
ValueFrom: apprepo.Spec.Auth.Header,
})
}
return envVars
}

// apprepoCleanupJobArgs returns a list of args for the repo cleanup container
func apprepoCleanupJobArgs(repoName string) []string {
return []string{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ limitations under the License.
package v1alpha1

import (
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

Expand All @@ -35,9 +36,15 @@ type AppRepository struct {

// AppRepositorySpec is the spec for an AppRepository resource
type AppRepositorySpec struct {
Type string `json:"type"`
URL string `json:"url"`
ResyncRequests uint `json:"resyncRequests"`
Type string `json:"type"`
URL string `json:"url"`
Auth AppRepositoryAuth `json:"auth,omitempty"`
ResyncRequests uint `json:"resyncRequests"`
}

// AppRepositoryAuth is the auth for an AppRepository resource
type AppRepositoryAuth struct {
Header *corev1.EnvVarSource `json:"header,omitempty"`
}

// AppRepositoryStatus is the status for an AppRepository resource
Expand Down
3 changes: 2 additions & 1 deletion cmd/chart-repo/sync.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,8 @@ var syncCmd = &cobra.Command{
logrus.Fatalf("Can't connect to mongoDB: %v", err)
}

if err = syncRepo(dbSession, args[0], args[1]); err != nil {
authorizationHeader := os.Getenv("AUTHORIZATION_HEADER")
if err = syncRepo(dbSession, args[0], args[1], authorizationHeader); err != nil {
logrus.Fatalf("Can't add chart repository to database: %v", err)
}

Expand Down
5 changes: 3 additions & 2 deletions cmd/chart-repo/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,9 @@ import (
)

type repo struct {
Name string
URL string
Name string
URL string
AuthorizationHeader string `bson:"-"`
}

type maintainer struct {
Expand Down
33 changes: 23 additions & 10 deletions cmd/chart-repo/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,18 +73,20 @@ func parseRepoUrl(repoURL string) (*url.URL, error) {
// These steps are processed in this way to ensure relevant chart data is
// imported into the database as fast as possible. E.g. we want all icons for
// charts before fetching readmes for each chart and version pair.
func syncRepo(dbSession datastore.Session, repoName, repoURL string) error {
func syncRepo(dbSession datastore.Session, repoName, repoURL string, authorizationHeader string) error {
url, err := parseRepoUrl(repoURL)
if err != nil {
log.WithFields(log.Fields{"url": repoURL}).WithError(err).Error("failed to parse URL")
return err
}
index, err := fetchRepoIndex(url)

r := repo{Name: repoName, URL: url.String(), AuthorizationHeader: authorizationHeader}
index, err := fetchRepoIndex(r)
if err != nil {
return err
}

charts := chartsFromIndex(index, repo{Name: repoName, URL: url.String()})
charts := chartsFromIndex(index, r)
err = importCharts(dbSession, charts)
if err != nil {
return err
Expand Down Expand Up @@ -151,16 +153,22 @@ func deleteRepo(dbSession datastore.Session, repoName string) error {
return err
}

func fetchRepoIndex(repoURL *url.URL) (*helmrepo.IndexFile, error) {
// use a copy of the URL struct so we don't modify the original
indexURL := *repoURL
func fetchRepoIndex(r repo) (*helmrepo.IndexFile, error) {
indexURL, err := parseRepoUrl(r.URL)
if err != nil {
log.WithFields(log.Fields{"url": r.URL}).WithError(err).Error("failed to parse URL")
return nil, err
}
indexURL.Path = path.Join(indexURL.Path, "index.yaml")
req, err := http.NewRequest("GET", indexURL.String(), nil)
req.Header.Set("User-Agent", userAgent)
if err != nil {
log.WithFields(log.Fields{"url": req.URL.String()}).WithError(err).Error("could not build repo index request")
return nil, err
}
req.Header.Set("User-Agent", userAgent)
if len(r.AuthorizationHeader) > 0 {
req.Header.Set("Authorization", r.AuthorizationHeader)
}
res, err := netClient.Do(req)
if err != nil {
log.WithFields(log.Fields{"url": req.URL.String()}).WithError(err).Error("error requesting repo index")
Expand Down Expand Up @@ -264,10 +272,13 @@ func fetchAndImportIcon(dbSession datastore.Session, c chart) error {
}

req, err := http.NewRequest("GET", c.Icon, nil)
req.Header.Set("User-Agent", userAgent)
if err != nil {
return err
}
req.Header.Set("User-Agent", userAgent)
if len(c.Repo.AuthorizationHeader) > 0 {
req.Header.Set("Authorization", c.Repo.AuthorizationHeader)
}

res, err := netClient.Do(req)
if err != nil {
Expand Down Expand Up @@ -307,11 +318,13 @@ func fetchAndImportFiles(dbSession datastore.Session, name string, r repo, cv ch

url := chartTarballURL(r, cv)
req, err := http.NewRequest("GET", url, nil)

req.Header.Set("User-Agent", userAgent)
if err != nil {
return err
}
req.Header.Set("User-Agent", userAgent)
if len(r.AuthorizationHeader) > 0 {
req.Header.Set("Authorization", r.AuthorizationHeader)
}

res, err := netClient.Do(req)
if err != nil {
Expand Down
77 changes: 63 additions & 14 deletions cmd/chart-repo/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,19 @@ func (h *goodHTTPClient) Do(req *http.Request) (*http.Response, error) {
return w.Result(), nil
}

type authenticatedHTTPClient struct{}

func (h *authenticatedHTTPClient) Do(req *http.Request) (*http.Response, error) {
w := httptest.NewRecorder()

// Ensure we're sending the right Authorization header
if !strings.Contains(req.Header.Get("Authorization"), "Bearer ThisSecretAccessTokenAuthenticatesTheClient") {
w.WriteHeader(500)
}
w.Write([]byte(validRepoIndexYAML))
return w.Result(), nil
}

type badIconClient struct{}

func (h *badIconClient) Do(req *http.Request) (*http.Response, error) {
Expand Down Expand Up @@ -121,6 +134,27 @@ func (h *goodTarballClient) Do(req *http.Request) (*http.Response, error) {
return w.Result(), nil
}

type authenticatedTarballClient struct {
c chart
}

func (h *authenticatedTarballClient) Do(req *http.Request) (*http.Response, error) {
w := httptest.NewRecorder()

// Ensure we're sending the right Authorization header
if !strings.Contains(req.Header.Get("Authorization"), "Bearer ThisSecretAccessTokenAuthenticatesTheClient") {
w.WriteHeader(500)
} else {
gzw := gzip.NewWriter(w)
files := []tarballFile{{h.c.Name + "/Chart.yaml", "should be a Chart.yaml here..."}}
files = append(files, tarballFile{h.c.Name + "/values.yaml", testChartValues})
files = append(files, tarballFile{h.c.Name + "/README.md", testChartReadme})
createTestTarball(gzw, files)
gzw.Flush()
}
return w.Result(), nil
}

func Test_syncURLInvalidity(t *testing.T) {
tests := []struct {
name string
Expand All @@ -133,37 +167,41 @@ func Test_syncURLInvalidity(t *testing.T) {
dbSession := mockstore.NewMockSession(&m)
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
err := syncRepo(dbSession, "test", tt.repoURL)
err := syncRepo(dbSession, "test", tt.repoURL, "")
assert.ExistsErr(t, err, tt.name)
})
}
}

func Test_fetchRepoIndex(t *testing.T) {
tests := []struct {
name string
repoURL string
name string
r repo
}{
{"valid HTTP URL", "http://my.examplerepo.com"},
{"valid HTTPS URL", "https://my.examplerepo.com"},
{"valid trailing URL", "https://my.examplerepo.com/"},
{"valid subpath URL", "https://subpath.test/subpath/"},
{"valid URL with trailing spaces", "https://subpath.test/subpath/ "},
{"valid URL with leading spaces", " https://subpath.test/subpath/"},
{"valid HTTP URL", repo{URL: "http://my.examplerepo.com"}},
{"valid HTTPS URL", repo{URL: "https://my.examplerepo.com"}},
{"valid trailing URL", repo{URL: "https://my.examplerepo.com/"}},
{"valid subpath URL", repo{URL: "https://subpath.test/subpath/"}},
{"valid URL with trailing spaces", repo{URL: "https://subpath.test/subpath/ "}},
{"valid URL with leading spaces", repo{URL: " https://subpath.test/subpath/"}},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
netClient = &goodHTTPClient{}
url, _ := parseRepoUrl(tt.repoURL)
_, err := fetchRepoIndex(url)
_, err := fetchRepoIndex(tt.r)
assert.NoErr(t, err)
})
}

t.Run("authenticated request", func(t *testing.T) {
netClient = &authenticatedHTTPClient{}
_, err := fetchRepoIndex(repo{URL: "https://my.examplerepo.com", AuthorizationHeader: "Bearer ThisSecretAccessTokenAuthenticatesTheClient"})
assert.NoErr(t, err)
})

t.Run("failed request", func(t *testing.T) {
netClient = &badHTTPClient{}
url, _ := parseRepoUrl("https://my.examplerepo.com")
_, err := fetchRepoIndex(url)
_, err := fetchRepoIndex(repo{URL: "https://my.examplerepo.com"})
assert.ExistsErr(t, err, "failed request")
})
}
Expand Down Expand Up @@ -293,7 +331,7 @@ func Test_fetchAndImportIcon(t *testing.T) {

func Test_fetchAndImportFiles(t *testing.T) {
index, _ := parseRepoIndex([]byte(validRepoIndexYAML))
charts := chartsFromIndex(index, repo{Name: "test", URL: "http://testrepo.com"})
charts := chartsFromIndex(index, repo{Name: "test", URL: "http://testrepo.com", AuthorizationHeader: "Bearer ThisSecretAccessTokenAuthenticatesTheClient1s"})
cv := charts[0].ChartVersions[0]

t.Run("http error", func(t *testing.T) {
Expand All @@ -315,6 +353,17 @@ func Test_fetchAndImportFiles(t *testing.T) {
m.AssertExpectations(t)
})

t.Run("authenticated request", func(t *testing.T) {
netClient = &authenticatedTarballClient{c: charts[0]}
m := mock.Mock{}
m.On("One", mock.Anything).Return(errors.New("return an error when checking if files already exists to force fetching"))
m.On("Insert", chartFiles{fmt.Sprintf("%s/%s-%s", charts[0].Repo.Name, charts[0].Name, cv.Version), testChartReadme, testChartValues, charts[0].Repo})
dbSession := mockstore.NewMockSession(&m)
err := fetchAndImportFiles(dbSession, charts[0].Name, charts[0].Repo, cv)
assert.NoErr(t, err)
m.AssertExpectations(t)
})

t.Run("valid tarball", func(t *testing.T) {
netClient = &goodTarballClient{c: charts[0]}
m := mock.Mock{}
Expand Down
6 changes: 5 additions & 1 deletion dashboard/src/actions/charts.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { Dispatch } from "redux";
import { createAction, getReturnOfExpression } from "typesafe-actions";

import { AppRepository } from "../shared/AppRepository";
import Chart from "../shared/Chart";
import { IChart, IChartVersion, IStoreState } from "../shared/types";
import * as url from "../shared/url";
Expand Down Expand Up @@ -137,12 +138,14 @@ export function deployChart(
values?: string,
resourceVersion?: string,
) {
return (dispatch: Dispatch<IStoreState>): Promise<{}> => {
return async (dispatch: Dispatch<IStoreState>): Promise<{}> => {
const chartAttrs = chartVersion.relationships.chart.data;
const method = resourceVersion ? "PUT" : "POST";
const endpoint = resourceVersion
? url.api.helmreleases.upgrade(namespace, releaseName)
: url.api.helmreleases.create(namespace);
const repo = await AppRepository.get(chartAttrs.repo.name);
const auth = repo.spec.auth;
return fetch(endpoint, {
headers: { "Content-Type": "application/json" },
method,
Expand All @@ -158,6 +161,7 @@ export function deployChart(
resourceVersion,
},
spec: {
auth,
chartName: chartAttrs.name,
repoUrl: chartAttrs.repo.url,
values,
Expand Down
39 changes: 34 additions & 5 deletions dashboard/src/actions/repos.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@ import { createAction, getReturnOfExpression } from "typesafe-actions";

import { Dispatch } from "react-redux";
import { AppRepository } from "../shared/AppRepository";
import { IAppRepository, IStoreState } from "../shared/types";
import Secret from "../shared/Secret";

import { IAppRepository, IOwnerReference, IStoreState } from "../shared/types";

export const addRepo = createAction("ADD_REPO");
export const addedRepo = createAction("ADDED_REPO", (added: IAppRepository) => ({
Expand Down Expand Up @@ -80,11 +82,38 @@ export const fetchRepos = () => {
};
};

export const installRepo = (name: string, url: string) => {
export const installRepo = (name: string, url: string, authHeader: string) => {
return async (dispatch: Dispatch<IStoreState>) => {
let auth;
const secretName = `apprepo-${name}-secrets`;
if (authHeader.length) {
auth = {
header: {
secretKeyRef: {
key: "authorizationHeader",
name: secretName,
},
},
};
}
dispatch(addRepo());
const added = await AppRepository.create(name, url);
dispatch(addedRepo(added));
return added;
const apprepo = await AppRepository.create(name, url, auth);
dispatch(addedRepo(apprepo));

if (authHeader.length) {
await Secret.create(
secretName,
{ authorizationHeader: btoa(authHeader) },
{
apiVersion: apprepo.apiVersion,
blockOwnerDeletion: true,
kind: apprepo.kind,
name: apprepo.metadata.name,
uid: apprepo.metadata.uid,
} as IOwnerReference,
"kubeapps",
);
}
return apprepo;
};
};
Loading

0 comments on commit 626bb52

Please sign in to comment.