diff --git a/api/handlers/app.go b/api/handlers/app.go index 5ec7f742d..30e7b0647 100644 --- a/api/handlers/app.go +++ b/api/handlers/app.go @@ -50,7 +50,6 @@ const ( type CFAppRepository interface { GetApp(context.Context, authorization.Info, string) (repositories.AppRecord, error) ListApps(context.Context, authorization.Info, repositories.ListAppsMessage) ([]repositories.AppRecord, error) - GetAppEnvVars(context.Context, authorization.Info, string) (repositories.AppEnvVarsRecord, error) PatchAppEnvVars(context.Context, authorization.Info, repositories.PatchAppEnvVarsMessage) (repositories.AppEnvVarsRecord, error) CreateApp(context.Context, authorization.Info, repositories.CreateAppMessage) (repositories.AppRecord, error) SetCurrentDroplet(context.Context, authorization.Info, repositories.SetCurrentDropletMessage) (repositories.CurrentDropletRecord, error) @@ -486,11 +485,16 @@ func (h *App) getEnvVars(r *http.Request) (*routing.Response, error) { logger := logr.FromContextOrDiscard(r.Context()).WithName("handlers.app.get-env-vars") appGUID := routing.URLParam(r, "guid") - envVarsRecord, err := h.appRepo.GetAppEnvVars(r.Context(), authInfo, appGUID) + appEnvRecord, err := h.appRepo.GetAppEnv(r.Context(), authInfo, appGUID) if err != nil { return nil, apierrors.LogAndReturn(logger, err, "Failed to fetch app environment variables", "AppGUID", appGUID) } + envVarsRecord := repositories.AppEnvVarsRecord{ + AppGUID: appEnvRecord.AppGUID, + EnvironmentVariables: appEnvRecord.EnvironmentVariables, + } + return routing.NewResponse(http.StatusOK).WithBody(presenter.ForAppEnvVars(envVarsRecord, h.serverURL)), nil } diff --git a/api/handlers/app_test.go b/api/handlers/app_test.go index 6bb6d125d..2ad480dc1 100644 --- a/api/handlers/app_test.go +++ b/api/handlers/app_test.go @@ -1498,7 +1498,8 @@ var _ = Describe("App", func() { Describe("GET /v3/apps/:guid/environment_variables", func() { BeforeEach(func() { - appRepo.GetAppEnvVarsReturns(repositories.AppEnvVarsRecord{ + appRepo.GetAppEnvReturns(repositories.AppEnvRecord{ + AppGUID: appGUID, EnvironmentVariables: map[string]string{"VAR": "VAL"}, }, nil) @@ -1506,8 +1507,8 @@ var _ = Describe("App", func() { }) It("returns the app environment variables", func() { - Expect(appRepo.GetAppEnvVarsCallCount()).To(Equal(1)) - _, actualAuthInfo, _ := appRepo.GetAppEnvVarsArgsForCall(0) + Expect(appRepo.GetAppEnvCallCount()).To(Equal(1)) + _, actualAuthInfo, _ := appRepo.GetAppEnvArgsForCall(0) Expect(actualAuthInfo).To(Equal(authInfo)) Expect(rr).To(HaveHTTPStatus(http.StatusOK)) @@ -1517,7 +1518,7 @@ var _ = Describe("App", func() { When("there is an error fetching the app env", func() { BeforeEach(func() { - appRepo.GetAppEnvVarsReturns(repositories.AppEnvVarsRecord{}, errors.New("unknown!")) + appRepo.GetAppEnvReturns(repositories.AppEnvRecord{}, errors.New("unknown!")) }) It("returns an error", func() { diff --git a/api/handlers/fake/cfapp_repository.go b/api/handlers/fake/cfapp_repository.go index 73accdeb2..f3bab1ad0 100644 --- a/api/handlers/fake/cfapp_repository.go +++ b/api/handlers/fake/cfapp_repository.go @@ -69,21 +69,6 @@ type CFAppRepository struct { result1 repositories.AppEnvRecord result2 error } - GetAppEnvVarsStub func(context.Context, authorization.Info, string) (repositories.AppEnvVarsRecord, error) - getAppEnvVarsMutex sync.RWMutex - getAppEnvVarsArgsForCall []struct { - arg1 context.Context - arg2 authorization.Info - arg3 string - } - getAppEnvVarsReturns struct { - result1 repositories.AppEnvVarsRecord - result2 error - } - getAppEnvVarsReturnsOnCall map[int]struct { - result1 repositories.AppEnvVarsRecord - result2 error - } ListAppsStub func(context.Context, authorization.Info, repositories.ListAppsMessage) ([]repositories.AppRecord, error) listAppsMutex sync.RWMutex listAppsArgsForCall []struct { @@ -424,72 +409,6 @@ func (fake *CFAppRepository) GetAppEnvReturnsOnCall(i int, result1 repositories. }{result1, result2} } -func (fake *CFAppRepository) GetAppEnvVars(arg1 context.Context, arg2 authorization.Info, arg3 string) (repositories.AppEnvVarsRecord, error) { - fake.getAppEnvVarsMutex.Lock() - ret, specificReturn := fake.getAppEnvVarsReturnsOnCall[len(fake.getAppEnvVarsArgsForCall)] - fake.getAppEnvVarsArgsForCall = append(fake.getAppEnvVarsArgsForCall, struct { - arg1 context.Context - arg2 authorization.Info - arg3 string - }{arg1, arg2, arg3}) - stub := fake.GetAppEnvVarsStub - fakeReturns := fake.getAppEnvVarsReturns - fake.recordInvocation("GetAppEnvVars", []interface{}{arg1, arg2, arg3}) - fake.getAppEnvVarsMutex.Unlock() - if stub != nil { - return stub(arg1, arg2, arg3) - } - if specificReturn { - return ret.result1, ret.result2 - } - return fakeReturns.result1, fakeReturns.result2 -} - -func (fake *CFAppRepository) GetAppEnvVarsCallCount() int { - fake.getAppEnvVarsMutex.RLock() - defer fake.getAppEnvVarsMutex.RUnlock() - return len(fake.getAppEnvVarsArgsForCall) -} - -func (fake *CFAppRepository) GetAppEnvVarsCalls(stub func(context.Context, authorization.Info, string) (repositories.AppEnvVarsRecord, error)) { - fake.getAppEnvVarsMutex.Lock() - defer fake.getAppEnvVarsMutex.Unlock() - fake.GetAppEnvVarsStub = stub -} - -func (fake *CFAppRepository) GetAppEnvVarsArgsForCall(i int) (context.Context, authorization.Info, string) { - fake.getAppEnvVarsMutex.RLock() - defer fake.getAppEnvVarsMutex.RUnlock() - argsForCall := fake.getAppEnvVarsArgsForCall[i] - return argsForCall.arg1, argsForCall.arg2, argsForCall.arg3 -} - -func (fake *CFAppRepository) GetAppEnvVarsReturns(result1 repositories.AppEnvVarsRecord, result2 error) { - fake.getAppEnvVarsMutex.Lock() - defer fake.getAppEnvVarsMutex.Unlock() - fake.GetAppEnvVarsStub = nil - fake.getAppEnvVarsReturns = struct { - result1 repositories.AppEnvVarsRecord - result2 error - }{result1, result2} -} - -func (fake *CFAppRepository) GetAppEnvVarsReturnsOnCall(i int, result1 repositories.AppEnvVarsRecord, result2 error) { - fake.getAppEnvVarsMutex.Lock() - defer fake.getAppEnvVarsMutex.Unlock() - fake.GetAppEnvVarsStub = nil - if fake.getAppEnvVarsReturnsOnCall == nil { - fake.getAppEnvVarsReturnsOnCall = make(map[int]struct { - result1 repositories.AppEnvVarsRecord - result2 error - }) - } - fake.getAppEnvVarsReturnsOnCall[i] = struct { - result1 repositories.AppEnvVarsRecord - result2 error - }{result1, result2} -} - func (fake *CFAppRepository) ListApps(arg1 context.Context, arg2 authorization.Info, arg3 repositories.ListAppsMessage) ([]repositories.AppRecord, error) { fake.listAppsMutex.Lock() ret, specificReturn := fake.listAppsReturnsOnCall[len(fake.listAppsArgsForCall)] @@ -831,8 +750,6 @@ func (fake *CFAppRepository) Invocations() map[string][][]interface{} { defer fake.getAppMutex.RUnlock() fake.getAppEnvMutex.RLock() defer fake.getAppEnvMutex.RUnlock() - fake.getAppEnvVarsMutex.RLock() - defer fake.getAppEnvVarsMutex.RUnlock() fake.listAppsMutex.RLock() defer fake.listAppsMutex.RUnlock() fake.patchAppMutex.RLock() diff --git a/api/repositories/app_repository.go b/api/repositories/app_repository.go index a1fba41a2..7c2c75268 100644 --- a/api/repositories/app_repository.go +++ b/api/repositories/app_repository.go @@ -387,36 +387,6 @@ func (f *AppRepo) ListApps(ctx context.Context, authInfo authorization.Info, mes return f.sorter.Sort(slices.Collect(appRecords), message.OrderBy), nil } -func (f *AppRepo) GetAppEnvVars(ctx context.Context, authInfo authorization.Info, appGUID string) (AppEnvVarsRecord, error) { - app, err := f.GetApp(ctx, authInfo, appGUID) - if err != nil { - return AppEnvVarsRecord{}, err - } - - userClient, err := f.userClientFactory.BuildClient(authInfo) - if err != nil { - return AppEnvVarsRecord{}, fmt.Errorf("failed to build user client: %w", err) - } - - appEnvVarMap := map[string]string{} - if app.envSecretName != "" { - appEnvVarSecret := new(corev1.Secret) - err = userClient.Get(ctx, types.NamespacedName{Name: app.envSecretName, Namespace: app.SpaceGUID}, appEnvVarSecret) - if err != nil { - return AppEnvVarsRecord{}, fmt.Errorf("error finding environment variable Secret %q for App %q: %w", - app.envSecretName, - app.GUID, - apierrors.FromK8sError(err, AppEnvResourceType)) - } - appEnvVarMap = convertByteSliceValuesToStrings(appEnvVarSecret.Data) - } - - return AppEnvVarsRecord{ - AppGUID: app.GUID, - EnvironmentVariables: appEnvVarMap, - }, nil -} - func (f *AppRepo) PatchAppEnvVars(ctx context.Context, authInfo authorization.Info, message PatchAppEnvVarsMessage) (AppEnvVarsRecord, error) { secretObj := corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ diff --git a/api/repositories/app_repository_test.go b/api/repositories/app_repository_test.go index 84c0d00de..5671db677 100644 --- a/api/repositories/app_repository_test.go +++ b/api/repositories/app_repository_test.go @@ -734,97 +734,6 @@ var _ = Describe("AppRepository", func() { }) }) - Describe("GetAppEnvVars", func() { - var ( - envVars map[string]string - secretName string - appGUID string - appEnvVarsRecord repositories.AppEnvVarsRecord - getAppEnvVarsErr error - ) - - BeforeEach(func() { - appGUID = cfApp.Name - secretName = "the-env-secret" - - envVars = map[string]string{ - "RAILS_ENV": "production", - "LUNCHTIME": "12:00", - } - - secret := &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: secretName, - Namespace: cfSpace.Name, - }, - StringData: envVars, - } - - Expect(k8sClient.Create(ctx, secret)).To(Succeed()) - }) - - JustBeforeEach(func() { - appEnvVarsRecord, getAppEnvVarsErr = appRepo.GetAppEnvVars(ctx, authInfo, appGUID) - }) - - When("the user can read secrets in the space", func() { - BeforeEach(func() { - Expect(k8s.PatchResource(ctx, k8sClient, cfApp, func() { - cfApp.Spec.EnvSecretName = secretName - })).To(Succeed()) - - createRoleBinding(ctx, userName, spaceDeveloperRole.Name, cfSpace.Name) - }) - - It("returns the environment variables stored on the secret", func() { - Expect(getAppEnvVarsErr).NotTo(HaveOccurred()) - Expect(appEnvVarsRecord.EnvironmentVariables).To(Equal(envVars)) - }) - - When("the EnvSecret doesn't exist", func() { - BeforeEach(func() { - secretName = "doIReallyExist" - Expect(k8s.PatchResource(ctx, k8sClient, cfApp, func() { - cfApp.Spec.EnvSecretName = secretName - })).To(Succeed()) - }) - - It("errors", func() { - Expect(getAppEnvVarsErr).To(MatchError(ContainSubstring("Secret"))) - }) - }) - }) - - When("EnvSecretName is blank", func() { - BeforeEach(func() { - secretName = "" - Expect(k8s.PatchResource(ctx, k8sClient, cfApp, func() { - cfApp.Spec.EnvSecretName = secretName - })).To(Succeed()) - }) - - It("returns an empty map", func() { - Expect(appEnvVarsRecord.EnvironmentVariables).To(BeEmpty()) - }) - }) - - When("the user doesn't have permission to get secrets in the space", func() { - It("errors", func() { - Expect(getAppEnvVarsErr).To(matchers.WrapErrorAssignableToTypeOf(apierrors.ForbiddenError{})) - }) - }) - - When("the app does not exist", func() { - BeforeEach(func() { - appGUID = "i don't exist" - }) - It("returns an error", func() { - Expect(getAppEnvVarsErr).To(HaveOccurred()) - Expect(getAppEnvVarsErr).To(matchers.WrapErrorAssignableToTypeOf(apierrors.NotFoundError{})) - }) - }) - }) - Describe("PatchAppEnvVars", func() { const ( key0 = "KEY0"