Skip to content

Commit

Permalink
Simplify code by using existing GetAppEnv function
Browse files Browse the repository at this point in the history
  • Loading branch information
gogolok committed Nov 3, 2024
1 parent feb5744 commit 4885969
Show file tree
Hide file tree
Showing 5 changed files with 12 additions and 211 deletions.
10 changes: 7 additions & 3 deletions api/handlers/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -486,12 +485,17 @@ 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)
}

return routing.NewResponse(http.StatusOK).WithBody(presenter.ForAppEnvVars(envVarsRecord, h.serverURL)), nil
appEnvVarsRecord := repositories.AppEnvVarsRecord{
AppGUID: appEnvRecord.AppGUID,
EnvironmentVariables: appEnvRecord.EnvironmentVariables,
}

return routing.NewResponse(http.StatusOK).WithBody(presenter.ForAppEnvVars(appEnvVarsRecord, h.serverURL)), nil
}

func (h *App) updateEnvVars(r *http.Request) (*routing.Response, error) {
Expand Down
9 changes: 5 additions & 4 deletions api/handlers/app_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1498,16 +1498,17 @@ 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)

req = createHttpRequest("GET", "/v3/apps/"+appGUID+"/environment_variables", nil)
})

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))
Expand All @@ -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() {
Expand Down
83 changes: 0 additions & 83 deletions api/handlers/fake/cfapp_repository.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

30 changes: 0 additions & 30 deletions api/repositories/app_repository.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
91 changes: 0 additions & 91 deletions api/repositories/app_repository_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down

0 comments on commit 4885969

Please sign in to comment.