Switch dashboard to use backend API for creating app repos (#1422)

* Switch dashboard to use backend API for creating app repos. * Update apprepos handler to return apprepository as a property (and dashboard to match) * Update chart version. * Use POD_NAME from env and don't update chart version.
vmware-tanzu · Jan 10, 2020 · 1b19c36 · 1b19c36
1 parent a7f9b5e
commit 1b19c36
Show file tree

Hide file tree

Showing 9 changed files with 82 additions and 118 deletions.
diff --git a/cmd/tiller-proxy/internal/handler/apprepos_handler.go b/cmd/tiller-proxy/internal/handler/apprepos_handler.go
@@ -76,13 +76,18 @@ type appRepositoryRequest struct {
 
 type appRepositoryRequestDetails struct {
 	Name               string                 `json:"name"`
-	RepoURL            string                 `json:"repoUrl"`
+	RepoURL            string                 `json:"repoURL"`
 	AuthHeader         string                 `json:"authHeader"`
 	CustomCA           string                 `json:"customCA"`
 	SyncJobPodTemplate corev1.PodTemplateSpec `json:"syncJobPodTemplate"`
 	ResyncRequests     uint                   `json:"resyncRequests"`
 }
 
+// appRepositoryResponse is used to marshal the JSON response
+type appRepositoryResponse struct {
+	AppRepository v1alpha1.AppRepository `json:"appRepository"`
+}
+
 // NewAppRepositoriesHandler returns an AppRepositories handler configured with
 // the in-cluster config but overriding the token with an empty string, so that
 // ConfigForToken must be called to obtain a valid config.
@@ -137,6 +142,7 @@ func (a *appRepositoriesHandler) Create(w http.ResponseWriter, req *http.Request
 	if a.kubeappsNamespace == "" {
 		log.Errorf("attempt to use app repositories handler without kubeappsNamespace configured")
 		http.Error(w, "kubeappsNamespace must be configured to enable app repository handler", http.StatusUnauthorized)
+		return
 	}
 
 	token := auth.ExtractToken(req.Header.Get("Authorization"))
@@ -190,7 +196,15 @@ func (a *appRepositoriesHandler) Create(w http.ResponseWriter, req *http.Request
 	}
 
 	w.WriteHeader(http.StatusCreated)
-	w.Write([]byte("OK"))
+	response := appRepositoryResponse{
+		AppRepository: *appRepo,
+	}
+	responseBody, err := json.Marshal(response)
+	if err != nil {
+		http.Error(w, err.Error(), http.StatusInternalServerError)
+		return
+	}
+	w.Write(responseBody)
 }
 
 // appRepositoryForRequest takes care of parsing the request data into an AppRepository.
@@ -256,8 +270,8 @@ func secretForRequest(appRepoRequest appRepositoryRequest, appRepo *v1alpha1.App
 			Name: secretNameForRepo(appRepo.Name),
 			OwnerReferences: []metav1.OwnerReference{
 				metav1.OwnerReference{
-					APIVersion:         appRepo.TypeMeta.APIVersion,
-					Kind:               appRepo.TypeMeta.Kind,
+					APIVersion:         "kubeapps.com/v1alpha1",
+					Kind:               "AppRepository",
 					Name:               appRepo.ObjectMeta.Name,
 					UID:                appRepo.ObjectMeta.UID,
 					BlockOwnerDeletion: &blockOwnerDeletion,

diff --git a/cmd/tiller-proxy/internal/handler/apprepos_handler_test.go b/cmd/tiller-proxy/internal/handler/apprepos_handler_test.go
@@ -79,6 +79,12 @@ func TestAppRepositoryCreate(t *testing.T) {
 			requestData:       `{"appRepository": {"name": "test-repo", "url": "http://example.com/test-repo"}}`,
 			expectedCode:      http.StatusCreated,
 		},
+		{
+			name:              "it creates an app repository with an empty template",
+			kubeappsNamespace: "kubeapps",
+			requestData:       `{"appRepository": {"name": "test-repo", "url": "http://example.com/test-repo", "syncJobPodTemplate": {}}}`,
+			expectedCode:      http.StatusCreated,
+		},
 		{
 			name:              "it errors if the repo exists in the kubeapps ns already",
 			kubeappsNamespace: "kubeapps",
@@ -159,6 +165,17 @@ func TestAppRepositoryCreate(t *testing.T) {
 					t.Errorf("mismatch (-want +got):\n%s", cmp.Diff(want, got))
 				}
 
+				// Ensure the response contained the created app repository
+				var appRepoResponse appRepositoryResponse
+				err = json.NewDecoder(response.Body).Decode(&appRepoResponse)
+				if err != nil {
+					t.Fatalf("%+v", err)
+				}
+				expectedResponse := appRepositoryResponse{AppRepository: *requestAppRepo}
+				if got, want := appRepoResponse, expectedResponse; !cmp.Equal(want, got) {
+					t.Errorf("mismatch (-want +got):\n%s", cmp.Diff(want, got))
+				}
+
 				// When appropriate, ensure the expected secret is stored
 				if appRepoRequest.AppRepository.AuthHeader != "" {
 					requestSecret := secretForRequest(appRepoRequest, responseAppRepo)
@@ -344,7 +361,7 @@ func TestSecretForRequest(t *testing.T) {
 	blockOwnerDeletion := true
 	ownerRefs := []metav1.OwnerReference{
 		metav1.OwnerReference{
-			APIVersion:         "v1",
+			APIVersion:         "kubeapps.com/v1alpha1",
 			Kind:               "AppRepository",
 			Name:               "test-repo",
 			UID:                "abcd1234",

diff --git a/cmd/tiller-proxy/main.go b/cmd/tiller-proxy/main.go
@@ -63,8 +63,7 @@ var (
 	tlsCertDefault   = fmt.Sprintf("%s/tls.crt", os.Getenv("HELM_HOME"))
 	tlsKeyDefault    = fmt.Sprintf("%s/tls.key", os.Getenv("HELM_HOME"))
 
-	assetsvcURL       string
-	kubeappsNamespace string
+	assetsvcURL string
 )
 
 func init() {
@@ -81,8 +80,6 @@ func init() {
 	// Default timeout from https://github.com/helm/helm/blob/b0b0accdfc84e154b3d48ec334cd5b4f9b345667/cmd/helm/install.go#L216
 	pflag.Int64Var(&timeout, "timeout", 300, "Timeout to perform release operations (install, upgrade, rollback, delete)")
 	pflag.StringVar(&assetsvcURL, "assetsvc-url", "http://kubeapps-internal-assetsvc:8080", "URL to the internal assetsvc")
-	// kubeapps-namespace is required only for the app repository handler which may move in the future.
-	pflag.StringVar(&kubeappsNamespace, "kubeapps-namespace", "", "namespace in which Kubeapps is running")
 }
 
 func main() {
@@ -186,7 +183,7 @@ func main() {
 	// Backend routes unrelated to tiller-proxy functionality.
 	// TODO(mnelson): Once the helm3 support is complete and tiller-proxy is being removed,
 	// reconsider where these endpoints live.
-	appreposHandler, err := handler.NewAppRepositoriesHandler(kubeappsNamespace)
+	appreposHandler, err := handler.NewAppRepositoriesHandler(os.Getenv("POD_NAMESPACE"))
 	if err != nil {
 		log.Fatalf("Unable to create app repositories handler: %+v", err)
 	}

diff --git a/dashboard/src/actions/repos.test.tsx b/dashboard/src/actions/repos.test.tsx
@@ -25,7 +25,7 @@ beforeEach(() => {
   });
   AppRepository.update = jest.fn();
   AppRepository.create = jest.fn().mockImplementationOnce(() => {
-    return { metadata: { name: "repo-abc" } };
+    return { appRepository: { metadata: { name: "repo-abc" } } };
   });
   Secret.create = jest.fn();
 });
@@ -195,24 +195,20 @@ describe("installRepo", () => {
       "",
     );
 
-    const authStruct = {
-      header: { secretKeyRef: { key: "authorizationHeader", name: "apprepo-my-repo-secrets" } },
-    };
-
     it("calls AppRepository create including a auth struct", async () => {
       await store.dispatch(installRepoCMDAuth);
       expect(AppRepository.create).toHaveBeenCalledWith(
         "my-repo",
-        "my-namespace",
         "http://foo.bar",
-        authStruct,
+        "Bearer: abc",
+        "",
         {},
       );
     });
 
-    it("creates the K8s secret", async () => {
+    it("does not create the K8s secret as API includes this", async () => {
       await store.dispatch(installRepoCMDAuth);
-      expect(Secret.create).toHaveBeenCalled();
+      expect(Secret.create).not.toHaveBeenCalled();
     });
 
     it("returns true", async () => {
@@ -230,24 +226,20 @@ describe("installRepo", () => {
       "",
     );
 
-    const authStruct = {
-      customCA: { secretKeyRef: { key: "ca.crt", name: "apprepo-my-repo-secrets" } },
-    };
-
     it("calls AppRepository create including a auth struct", async () => {
       await store.dispatch(installRepoCMDAuth);
       expect(AppRepository.create).toHaveBeenCalledWith(
         "my-repo",
-        "my-namespace",
         "http://foo.bar",
-        authStruct,
+        "",
+        "This is a cert!",
         {},
       );
     });
 
-    it("creates the K8s secret", async () => {
+    it("does not create the K8s secret as API includes this", async () => {
       await store.dispatch(installRepoCMDAuth);
-      expect(Secret.create).toHaveBeenCalled();
+      expect(Secret.create).not.toHaveBeenCalled();
     });
 
     it("returns true", async () => {
@@ -269,13 +261,9 @@ spec:
           repoActions.installRepo("my-repo", "http://foo.bar", "", "", safeYAMLTemplate),
         );
 
-        expect(AppRepository.create).toHaveBeenCalledWith(
-          "my-repo",
-          "my-namespace",
-          "http://foo.bar",
-          {},
-          { spec: { containers: [{ env: [{ name: "FOO", value: "BAR" }] }] } },
-        );
+        expect(AppRepository.create).toHaveBeenCalledWith("my-repo", "http://foo.bar", "", "", {
+          spec: { containers: [{ env: [{ name: "FOO", value: "BAR" }] }] },
+        });
       });
 
       // Example from https://nealpoole.com/blog/2013/06/code-execution-via-yaml-in-js-yaml-nodejs-module/
@@ -294,18 +282,7 @@ spec:
   context("when authHeader and customCA are empty", () => {
     it("calls AppRepository create without a auth struct", async () => {
       await store.dispatch(installRepoCMD);
-      expect(AppRepository.create).toHaveBeenCalledWith(
-        "my-repo",
-        "my-namespace",
-        "http://foo.bar",
-        {},
-        {},
-      );
-    });
-
-    it("does not create a K8s secret", async () => {
-      await store.dispatch(installRepoCMD);
-      expect(Secret.create).not.toHaveBeenCalled();
+      expect(AppRepository.create).toHaveBeenCalledWith("my-repo", "http://foo.bar", "", "", {});
     });
 
     it("returns true", async () => {

diff --git a/dashboard/src/actions/repos.ts b/dashboard/src/actions/repos.ts
@@ -3,10 +3,9 @@ import { ThunkAction } from "redux-thunk";
 import { ActionType, createAction } from "typesafe-actions";
 import { AppRepository } from "../shared/AppRepository";
 import Chart from "../shared/Chart";
-import Secret from "../shared/Secret";
 import { errorChart } from "./charts";
 
-import { IAppRepository, IOwnerReference, IStoreState, NotFoundError } from "../shared/types";
+import { IAppRepository, IStoreState, NotFoundError } from "../shared/types";
 
 export const addRepo = createAction("ADD_REPO");
 export const addedRepo = createAction("ADDED_REPO", resolve => {
@@ -131,69 +130,21 @@ export const installRepo = (
   syncJobPodTemplate: string,
 ): ThunkAction<Promise<boolean>, IStoreState, null, AppReposAction> => {
   return async (dispatch, getState) => {
+    let syncJobPodTemplateObj = {};
     try {
-      const {
-        config: { namespace },
-      } = getState();
-      interface ISecretKeyRef {
-        key: string;
-        name: string;
-      }
-      const auth: {
-        header?: { secretKeyRef: ISecretKeyRef };
-        customCA?: { secretKeyRef: ISecretKeyRef };
-      } = {};
-      const secrets: { [s: string]: string } = {};
-      const secretName = `apprepo-${name}-secrets`;
-      if (authHeader.length || customCA.length) {
-        // ensure we can create secrets in the kubeapps namespace
-        if (authHeader.length) {
-          auth.header = {
-            secretKeyRef: {
-              key: "authorizationHeader",
-              name: secretName,
-            },
-          };
-          secrets.authorizationHeader = btoa(authHeader);
-        }
-        if (customCA.length) {
-          auth.customCA = {
-            secretKeyRef: {
-              key: "ca.crt",
-              name: secretName,
-            },
-          };
-          secrets["ca.crt"] = btoa(customCA);
-        }
-      }
-      let syncJobPodTemplateObj = {};
       if (syncJobPodTemplate.length) {
         syncJobPodTemplateObj = yaml.safeLoad(syncJobPodTemplate);
       }
       dispatch(addRepo());
-      const apprepo = await AppRepository.create(
+      const data = await AppRepository.create(
         name,
-        namespace,
         repoURL,
-        auth,
+        authHeader,
+        customCA,
         syncJobPodTemplateObj,
       );
-      dispatch(addedRepo(apprepo));
-
-      if (authHeader.length || customCA.length) {
-        await Secret.create(
-          secretName,
-          secrets,
-          {
-            apiVersion: apprepo.apiVersion,
-            blockOwnerDeletion: true,
-            kind: apprepo.kind,
-            name: apprepo.metadata.name,
-            uid: apprepo.metadata.uid,
-          } as IOwnerReference,
-          namespace,
-        );
-      }
+      dispatch(addedRepo(data.appRepository));
+
       return true;
     } catch (e) {
       dispatch(errorRepos(e, "create"));

diff --git a/dashboard/src/shared/AppRepository.ts b/dashboard/src/shared/AppRepository.ts
@@ -1,6 +1,7 @@
 import { axiosWithAuth } from "./AxiosInstance";
 import { APIBase } from "./Kube";
-import { IAppRepository, IAppRepositoryList } from "./types";
+import { IAppRepository, IAppRepositoryList, ICreateAppRepositoryResponse } from "./types";
+import * as url from "./url";
 
 export class AppRepository {
   public static async list(namespace: string) {
@@ -25,23 +26,19 @@ export class AppRepository {
     return data;
   }
 
+  // create uses the kubeapps backend API
+  // TODO(mnelson) Update other endpoints to similarly use the backend API, removing the need
+  // for direct k8s api access (for this resource, at least).
   public static async create(
     name: string,
-    namespace: string,
-    url: string,
-    auth: any,
+    repoURL: string,
+    authHeader: string,
+    customCA: string,
     syncJobPodTemplate: any,
   ) {
-    const { data } = await axiosWithAuth.post<IAppRepository>(
-      AppRepository.getResourceLink(namespace),
-      {
-        apiVersion: "kubeapps.com/v1alpha1",
-        kind: "AppRepository",
-        metadata: {
-          name,
-        },
-        spec: { auth, type: "helm", url, syncJobPodTemplate },
-      },
+    const { data } = await axiosWithAuth.post<ICreateAppRepositoryResponse>(
+      url.backend.apprepositories.create(),
+      { appRepository: { name, repoURL, authHeader, customCA, syncJobPodTemplate } },
     );
     return data;
   }

diff --git a/dashboard/src/shared/types.ts b/dashboard/src/shared/types.ts
@@ -290,6 +290,10 @@ export interface IAppRepository
     undefined
   > {}
 
+export interface ICreateAppRepositoryResponse {
+  appRepository: IAppRepository;
+}
+
 export interface IAppRepositoryList
   extends IK8sList<
     IAppRepository,

diff --git a/dashboard/src/shared/url.ts b/dashboard/src/shared/url.ts
@@ -10,6 +10,13 @@ export const app = {
   },
 };
 
+export const backend = {
+  apprepositories: {
+    base: "api/v1/apprepositories",
+    create: () => `${backend.apprepositories.base}`,
+  },
+};
+
 export const api = {
   apprepostories: {
     base: `${APIBase}/apis/kubeapps.com/v1alpha1`,

diff --git a/docs/user/manifests/kubeapps-local-dev-values.yaml b/docs/user/manifests/kubeapps-local-dev-values.yaml
@@ -2,7 +2,7 @@ frontend:
   replicaCount: 1
 tillerProxy:
   replicaCount: 1
-chartsvc:
+assetsvc:
   replicaCount: 1
 dashboard:
-  replicaCount: 1 
+  replicaCount: 1