Merge branch 'issues/458' of https://github.com/cloudfoundry/cf-k8s-c…

…ontrollers into issues/458

� Conflicts:
�	api/apis/integration/create_service_binding_test.go

api/actions/fetch_process_stats_test.go

@@ -70,11 +70,11 @@ var _ = Describe("GetProcessStatsAction", func() {
 		fetchProcessStatsAction = NewFetchProcessStats(processRepo, podRepo, appRepo)
 	})
 
-	When("on the happy path", func() {
-		BeforeEach(func() {
-			responseRecords, responseErr = fetchProcessStatsAction.Invoke(context.Background(), authInfo, processGUID)
-		})
+	JustBeforeEach(func() {
+		responseRecords, responseErr = fetchProcessStatsAction.Invoke(context.Background(), authInfo, processGUID)
+	})
 
+	When("on the happy path", func() {
 		It("does not return an error", func() {
 			Expect(responseErr).ToNot(HaveOccurred())
 		})
@@ -136,7 +136,6 @@ var _ = Describe("GetProcessStatsAction", func() {
 		When("GetProcess responds with some error", func() {
 			BeforeEach(func() {
 				processRepo.GetProcessReturns(repositories.ProcessRecord{}, errors.New("some-error"))
-				responseRecords, responseErr = fetchProcessStatsAction.Invoke(context.Background(), authInfo, processGUID)
 			})
 
 			It("returns a nil and error", func() {
@@ -148,7 +147,17 @@ var _ = Describe("GetProcessStatsAction", func() {
 		When("GetApp responds with some error", func() {
 			BeforeEach(func() {
 				appRepo.GetAppReturns(repositories.AppRecord{}, errors.New("some-error"))
-				responseRecords, responseErr = fetchProcessStatsAction.Invoke(context.Background(), authInfo, processGUID)
+			})
+
+			It("returns a nil and error", func() {
+				Expect(responseRecords).To(BeNil())
+				Expect(responseErr).To(MatchError("some-error"))
+			})
+		})
+
+		When("ListPodStats responds with some error", func() {
+			BeforeEach(func() {
+				podRepo.ListPodStatsReturns(nil, errors.New("some-error"))
 			})
 
 			It("returns a nil and error", func() {

api/apis/integration/app_test.go

@@ -39,7 +39,7 @@ var _ = Describe("App Handler", func() {
 		processRepo := repositories.NewProcessRepo(k8sClient, clientFactory)
 		routeRepo := repositories.NewRouteRepo(k8sClient, clientFactory)
 		domainRepo := repositories.NewDomainRepo(k8sClient, clientFactory)
-		podRepo := repositories.NewPodRepo(k8sClient)
+		podRepo := repositories.NewPodRepo(clientFactory)
 		orgRepo := repositories.NewOrgRepo(rootNamespace, k8sClient, clientFactory, nsPermissions, time.Minute, true)
 		scaleProcess := actions.NewScaleProcess(processRepo).Invoke
 		scaleAppProcess := actions.NewScaleAppProcess(appRepo, processRepo, scaleProcess).Invoke

api/apis/integration/get_app_env_test.go

@@ -28,7 +28,7 @@ var _ = Describe("GET /v3/apps/:guid/env", func() {
 		processRepo := repositories.NewProcessRepo(k8sClient, clientFactory)
 		routeRepo := repositories.NewRouteRepo(k8sClient, clientFactory)
 		dropletRepo := repositories.NewDropletRepo(k8sClient, clientFactory)
-		podRepo := repositories.NewPodRepo(k8sClient)
+		podRepo := repositories.NewPodRepo(clientFactory)
 		orgRepo := repositories.NewOrgRepo("root-ns", k8sClient, clientFactory, nsPermissions, time.Minute, true)
 		scaleProcess := actions.NewScaleProcess(processRepo).Invoke
 		scaleAppProcess := actions.NewScaleAppProcess(appRepo, processRepo, scaleProcess).Invoke

api/apis/process_handler.go

@@ -247,8 +247,8 @@ func (h *ProcessHandler) writeErrorResponse(w http.ResponseWriter, processGUID s
 		h.logger.Info(fmt.Sprintf("%s not found", tycerr.ResourceType), "ProcessGUID", processGUID)
 		writeNotFoundErrorResponse(w, tycerr.ResourceType)
 	case repositories.ForbiddenError:
-		h.logger.Info("Process forbidden", "ProcessGUID", processGUID)
-		writeNotFoundErrorResponse(w, "Process")
+		h.logger.Info(fmt.Sprintf("%s forbidden", tycerr.ResourceType()), "ProcessGUID", processGUID)
+		writeNotFoundErrorResponse(w, tycerr.ResourceType())
 	default:
 		h.logger.Error(err, "Failed to fetch process from Kubernetes", "ProcessGUID", processGUID)
 		writeUnknownErrorResponse(w)

api/apis/process_handler_test.go

@@ -174,7 +174,7 @@ var _ = Describe("ProcessHandler", func() {
 			})
 			When("the user lacks access", func() {
 				BeforeEach(func() {
-					processRepo.GetProcessReturns(repositories.ProcessRecord{}, repositories.NewForbiddenError(errors.New("access denied or something")))
+					processRepo.GetProcessReturns(repositories.ProcessRecord{}, repositories.NewForbiddenProcessError(errors.New("access denied or something")))
 				})
 
 				It("returns a not-found error", func() {
@@ -260,7 +260,7 @@ var _ = Describe("ProcessHandler", func() {
 
 		When("the process isn't accessible to the user", func() {
 			BeforeEach(func() {
-				processRepo.GetProcessReturns(repositories.ProcessRecord{}, repositories.ForbiddenError{})
+				processRepo.GetProcessReturns(repositories.ProcessRecord{}, repositories.NewForbiddenProcessError(nil))
 			})
 
 			It("returns an error", func() {
@@ -694,6 +694,36 @@ var _ = Describe("ProcessHandler", func() {
 				expectUnknownError()
 			})
 		})
+
+		When("the app is not authorized", func() {
+			BeforeEach(func() {
+				fetchProcessStats.Returns(nil, repositories.NewForbiddenAppError(nil))
+			})
+
+			It("returns an error", func() {
+				expectNotFoundError("App not found")
+			})
+		})
+
+		When("the process is not authorized", func() {
+			BeforeEach(func() {
+				fetchProcessStats.Returns(nil, repositories.NewForbiddenProcessError(nil))
+			})
+
+			It("returns an error", func() {
+				expectNotFoundError("Process not found")
+			})
+		})
+
+		When("the process stats are not authorized", func() {
+			BeforeEach(func() {
+				fetchProcessStats.Returns(nil, repositories.NewForbiddenProcessStatsError(nil))
+			})
+
+			It("returns an error", func() {
+				expectNotFoundError("Process stats not found")
+			})
+		})
 	})
 
 	Describe("the GET /v3/processes endpoint", func() {
@@ -886,7 +916,7 @@ var _ = Describe("ProcessHandler", func() {
 			"data": {
 			  "invocation_timeout": 2,
               "timeout": 5,
-              "endpoint": "http://myapp.com/health" 
+              "endpoint": "http://myapp.com/health"
 			},
 			"type": "port"
 		  }
@@ -1029,20 +1059,9 @@ var _ = Describe("ProcessHandler", func() {
 			})
 		})
 
-		When("user is not allowed to patch a process", func() {
-			BeforeEach(func() {
-				processRepo.PatchProcessReturns(repositories.ProcessRecord{}, repositories.NewForbiddenError(errors.New("nope")))
-				makePatchRequest(processGUID, validBody)
-			})
-
-			It("returns an unauthorised error", func() {
-				expectNotFoundError("Process not found")
-			})
-		})
-
 		When("user is not allowed to get a process", func() {
 			BeforeEach(func() {
-				processRepo.GetProcessReturns(repositories.ProcessRecord{}, repositories.NewForbiddenError(errors.New("nope")))
+				processRepo.GetProcessReturns(repositories.ProcessRecord{}, repositories.NewForbiddenProcessError(errors.New("nope")))
 				makePatchRequest(processGUID, validBody)
 			})
 

api/main.go

@@ -95,7 +95,7 @@ func main() {
 	orgRepo := repositories.NewOrgRepo(config.RootNamespace, privilegedCRClient, userClientFactory, nsPermissions, createTimeout, config.AuthEnabled)
 	appRepo := repositories.NewAppRepo(privilegedCRClient, userClientFactory, nsPermissions)
 	processRepo := repositories.NewProcessRepo(privilegedCRClient, userClientFactory)
-	podRepo := repositories.NewPodRepo(privilegedCRClient)
+	podRepo := repositories.NewPodRepo(userClientFactory)
 	dropletRepo := repositories.NewDropletRepo(privilegedCRClient, userClientFactory)
 	routeRepo := repositories.NewRouteRepo(privilegedCRClient, userClientFactory)
 	domainRepo := repositories.NewDomainRepo(privilegedCRClient, userClientFactory)

api/repositories/app_repository.go

@@ -177,7 +177,7 @@ func (f *AppRepo) GetApp(ctx context.Context, authInfo authorization.Info, appGU
 
 	err = userClient.Get(ctx, client.ObjectKey{Namespace: app.SpaceGUID, Name: app.GUID}, &workloadsv1alpha1.CFApp{})
 	if k8serrors.IsForbidden(err) {
-		return AppRecord{}, NewForbiddenError(err)
+		return AppRecord{}, NewForbiddenAppError(err)
 	}
 
 	if err != nil { // untested

api/repositories/errors.go

@@ -30,13 +30,26 @@ func (e NotFoundError) Unwrap() error {
 }
 
 type ForbiddenError struct {
-	err error
+	err          error
+	resourceType string
 }
 
 func NewForbiddenError(err error) ForbiddenError {
 	return ForbiddenError{err: err}
 }
 
+func NewForbiddenAppError(err error) ForbiddenError {
+	return ForbiddenError{err: err, resourceType: "App"}
+}
+
+func NewForbiddenProcessError(err error) ForbiddenError {
+	return ForbiddenError{err: err, resourceType: "Process"}
+}
+
+func NewForbiddenProcessStatsError(err error) ForbiddenError {
+	return ForbiddenError{err: err, resourceType: "Process stats"}
+}
+
 func (e ForbiddenError) Error() string {
 	return errMessage("Forbidden", e.err)
 }
@@ -45,6 +58,10 @@ func (e ForbiddenError) Unwrap() error {
 	return e.err
 }
 
+func (e ForbiddenError) ResourceType() string {
+	return e.resourceType
+}
+
 func IsForbiddenError(err error) bool {
 	return errors.As(err, &ForbiddenError{})
 }

api/repositories/pod_repository.go

@@ -2,12 +2,14 @@ package repositories
 
 import (
 	"context"
+	"fmt"
 	"strconv"
 
 	"code.cloudfoundry.org/cf-k8s-controllers/api/authorization"
 	workloadsv1alpha1 "code.cloudfoundry.org/cf-k8s-controllers/controllers/apis/workloads/v1alpha1"
 
 	corev1 "k8s.io/api/core/v1"
+	k8serrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/labels"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
@@ -28,11 +30,11 @@ const (
 )
 
 type PodRepo struct {
-	privilegedClient client.Client
+	userClientFactory UserK8sClientFactory
 }
 
-func NewPodRepo(privilegedClient client.Client) *PodRepo {
-	return &PodRepo{privilegedClient: privilegedClient}
+func NewPodRepo(userClientFactory UserK8sClientFactory) *PodRepo {
+	return &PodRepo{userClientFactory: userClientFactory}
 }
 
 type PodStatsRecord struct {
@@ -61,7 +63,7 @@ func (r *PodRepo) ListPodStats(ctx context.Context, authInfo authorization.Info,
 	}
 	listOpts := &client.ListOptions{Namespace: message.Namespace, LabelSelector: labelSelector}
 
-	pods, err := r.ListPods(ctx, authInfo, *listOpts)
+	pods, err := r.listPods(ctx, authInfo, *listOpts)
 	if err != nil {
 		return nil, err
 	}
@@ -88,12 +90,22 @@ func (r *PodRepo) ListPodStats(ctx context.Context, authInfo authorization.Info,
 	return records, nil
 }
 
-func (r *PodRepo) ListPods(ctx context.Context, authInfo authorization.Info, listOpts client.ListOptions) ([]corev1.Pod, error) {
+func (r *PodRepo) listPods(ctx context.Context, authInfo authorization.Info, listOpts client.ListOptions) ([]corev1.Pod, error) {
+	userClient, err := r.userClientFactory.BuildClient(authInfo)
+	if err != nil {
+		return nil, fmt.Errorf("failed to build user client: %w", err)
+	}
+
 	podList := corev1.PodList{}
-	err := r.privilegedClient.List(ctx, &podList, &listOpts)
+	err = userClient.List(ctx, &podList, &listOpts)
 	if err != nil {
-		return nil, err
+		if k8serrors.IsForbidden(err) {
+			return nil, NewForbiddenProcessStatsError(err)
+		}
+
+		return nil, fmt.Errorf("err in client.List: %w", err)
 	}
+
 	return podList.Items, nil
 }