From 52322936ce6083059f02e92555947e6f24b822aa Mon Sep 17 00:00:00 2001 From: Georgi Sabev Date: Tue, 2 Jul 2024 14:51:59 +0000 Subject: [PATCH] Implement `POST /v3/service_brokers` * Globally available brokers are only supported (space-restricted brokers are currently out of scope) * Introduce the `CFServiceBroker` custom resource * The `CFServiceBroker` controller sets its `Ready` state * Implement `POST /v3/service_brokers` * Only allow admin users to create brokers * The `CFServiceBroker` object is created in the root namespace (as it is globally available) * Implement `GET /v3/jobs/service_broker.create~` to return `COMPLETE` job state once the broker is ready * Validate broker name uniqueness via a validating webhook fixes #3263 Co-authored-by: Danail Branekov --- Makefile | 13 +- README.helm.md | 2 +- api/Dockerfile | 2 + .../fake/cfservice_broker_repository.go | 123 ++++++++++ api/handlers/fake/state_repository.go | 123 ++++++++++ api/handlers/job.go | 113 +++++++-- api/handlers/job_test.go | 88 ++++++- api/handlers/service_broker.go | 77 ++++++ api/handlers/service_broker_test.go | 115 +++++++++ api/main.go | 10 + api/payloads/service_broker.go | 42 ++++ api/payloads/service_broker_test.go | 113 +++++++++ api/presenter/job.go | 15 +- api/repositories/service_broker_repository.go | 134 ++++++++++ .../service_broker_repository_test.go | 229 ++++++++++++++++++ .../service_instance_repository.go | 18 +- controllers/Dockerfile | 1 + .../api/v1alpha1/cfservicebroker_types.go | 63 +++++ .../api/v1alpha1/cfserviceinstance_types.go | 3 +- controllers/api/v1alpha1/shared_types.go | 2 + .../api/v1alpha1/zz_generated.deepcopy.go | 98 ++++++++ .../services/brokers/controller.go | 146 +++++++++++ .../services/brokers/controller_test.go | 195 +++++++++++++++ .../services/brokers/suite_test.go | 92 +++++++ .../services/credentials/credentials.go | 13 + .../services/credentials/credentials_test.go | 23 ++ controllers/controllers/shared/index.go | 9 + controllers/main.go | 21 ++ .../webhooks/services/brokers/suite_test.go | 23 ++ .../webhooks/services/brokers/validator.go | 79 ++++++ .../services/brokers/validator_test.go | 158 ++++++++++++ .../korifi/controllers/cf_roles/cf_admin.yaml | 8 + ...ifi.cloudfoundry.org_cfservicebrokers.yaml | 160 ++++++++++++ helm/korifi/controllers/manifests.yaml | 22 ++ helm/korifi/controllers/role.yaml | 20 ++ helm/korifi/values.schema.json | 2 +- model/cfresource.go | 22 ++ model/metadata.go | 6 + model/services/brokers.go | 12 + model/services/zz_generated.deepcopy.go | 38 +++ model/zz_generated.deepcopy.go | 52 ++++ tests/e2e/e2e_suite_test.go | 11 + tests/e2e/service_brokers_test.go | 90 +++++++ 43 files changed, 2524 insertions(+), 62 deletions(-) create mode 100644 api/handlers/fake/cfservice_broker_repository.go create mode 100644 api/handlers/fake/state_repository.go create mode 100644 api/handlers/service_broker.go create mode 100644 api/handlers/service_broker_test.go create mode 100644 api/payloads/service_broker.go create mode 100644 api/payloads/service_broker_test.go create mode 100644 api/repositories/service_broker_repository.go create mode 100644 api/repositories/service_broker_repository_test.go create mode 100644 controllers/api/v1alpha1/cfservicebroker_types.go create mode 100644 controllers/controllers/services/brokers/controller.go create mode 100644 controllers/controllers/services/brokers/controller_test.go create mode 100644 controllers/controllers/services/brokers/suite_test.go create mode 100644 controllers/webhooks/services/brokers/suite_test.go create mode 100644 controllers/webhooks/services/brokers/validator.go create mode 100644 controllers/webhooks/services/brokers/validator_test.go create mode 100644 helm/korifi/controllers/crds/korifi.cloudfoundry.org_cfservicebrokers.yaml create mode 100644 model/cfresource.go create mode 100644 model/metadata.go create mode 100644 model/services/brokers.go create mode 100644 model/services/zz_generated.deepcopy.go create mode 100644 model/zz_generated.deepcopy.go create mode 100644 tests/e2e/service_brokers_test.go diff --git a/Makefile b/Makefile index 97b87693e..fa96bf811 100644 --- a/Makefile +++ b/Makefile @@ -25,13 +25,22 @@ help: ## Display this help. CONTROLLERS=controllers job-task-runner kpack-image-builder statefulset-runner COMPONENTS=api $(CONTROLLERS) -manifests: +manifests: install-controller-gen + $(CONTROLLER_GEN) \ + paths="./model/..." \ + crd \ + output:crd:artifacts:config=helm/korifi/controllers/crds @for comp in $(COMPONENTS); do make -C $$comp manifests; done -generate: +generate: install-controller-gen + $(CONTROLLER_GEN) object:headerFile="controllers/hack/boilerplate.go.txt" paths="./model/..." @for comp in $(CONTROLLERS); do make -C $$comp generate; done go run ./scripts/helmdoc/main.go > README.helm.md +CONTROLLER_GEN = $(shell pwd)/bin/controller-gen +install-controller-gen: + GOBIN=$(shell pwd)/bin go install sigs.k8s.io/controller-tools/cmd/controller-gen + generate-fakes: go generate ./... diff --git a/README.helm.md b/README.helm.md index 7dcb6d3ab..9299c2868 100644 --- a/README.helm.md +++ b/README.helm.md @@ -74,7 +74,7 @@ Here are all the values that can be set for the chart: - `debug` (_Boolean_): Enables remote debugging with [Delve](https://github.com/go-delve/delve). - `defaultAppDomainName` (_String_): Base domain name for application URLs. - `eksContainerRegistryRoleARN` (_String_): Amazon Resource Name (ARN) of the IAM role to use to access the ECR registry from an EKS deployed Korifi. Required if containerRegistrySecret not set. -- `experimental`: Experimental features. Make sure you do not enable those on production. No guarantee provided! Backwards incompatible changes in future are quite probable! +- `experimental`: Experimental features. No guarantees are provided and breaking/backwards incompatible changes should be expected. These features are not recommended for use in production environments. - `managedServices`: - `include` (_Boolean_): Enable managed services support - `generateIngressCertificates` (_Boolean_): Use `cert-manager` to generate self-signed certificates for the API and app endpoints. diff --git a/api/Dockerfile b/api/Dockerfile index 7f763b1e7..403003b37 100644 --- a/api/Dockerfile +++ b/api/Dockerfile @@ -10,6 +10,7 @@ COPY go.mod go.sum ./ RUN --mount=type=cache,target=/go/pkg/mod \ go mod download +COPY model model COPY api/actions api/actions COPY api/errors api/errors COPY api/authorization api/authorization @@ -26,6 +27,7 @@ COPY controllers/config controllers/config COPY controllers/controllers/shared controllers/controllers/shared COPY controllers/controllers/workloads controllers/controllers/workloads COPY controllers/controllers/services/credentials controllers/controllers/services/credentials +COPY controllers/controllers/services/brokers controllers/controllers/services/brokers COPY controllers/webhooks controllers/webhooks COPY tools tools COPY version version diff --git a/api/handlers/fake/cfservice_broker_repository.go b/api/handlers/fake/cfservice_broker_repository.go new file mode 100644 index 000000000..97e44f1b3 --- /dev/null +++ b/api/handlers/fake/cfservice_broker_repository.go @@ -0,0 +1,123 @@ +// Code generated by counterfeiter. DO NOT EDIT. +package fake + +import ( + "context" + "sync" + + "code.cloudfoundry.org/korifi/api/authorization" + "code.cloudfoundry.org/korifi/api/handlers" + "code.cloudfoundry.org/korifi/api/repositories" +) + +type CFServiceBrokerRepository struct { + CreateServiceBrokerStub func(context.Context, authorization.Info, repositories.CreateServiceBrokerMessage) (repositories.ServiceBrokerResource, error) + createServiceBrokerMutex sync.RWMutex + createServiceBrokerArgsForCall []struct { + arg1 context.Context + arg2 authorization.Info + arg3 repositories.CreateServiceBrokerMessage + } + createServiceBrokerReturns struct { + result1 repositories.ServiceBrokerResource + result2 error + } + createServiceBrokerReturnsOnCall map[int]struct { + result1 repositories.ServiceBrokerResource + result2 error + } + invocations map[string][][]interface{} + invocationsMutex sync.RWMutex +} + +func (fake *CFServiceBrokerRepository) CreateServiceBroker(arg1 context.Context, arg2 authorization.Info, arg3 repositories.CreateServiceBrokerMessage) (repositories.ServiceBrokerResource, error) { + fake.createServiceBrokerMutex.Lock() + ret, specificReturn := fake.createServiceBrokerReturnsOnCall[len(fake.createServiceBrokerArgsForCall)] + fake.createServiceBrokerArgsForCall = append(fake.createServiceBrokerArgsForCall, struct { + arg1 context.Context + arg2 authorization.Info + arg3 repositories.CreateServiceBrokerMessage + }{arg1, arg2, arg3}) + stub := fake.CreateServiceBrokerStub + fakeReturns := fake.createServiceBrokerReturns + fake.recordInvocation("CreateServiceBroker", []interface{}{arg1, arg2, arg3}) + fake.createServiceBrokerMutex.Unlock() + if stub != nil { + return stub(arg1, arg2, arg3) + } + if specificReturn { + return ret.result1, ret.result2 + } + return fakeReturns.result1, fakeReturns.result2 +} + +func (fake *CFServiceBrokerRepository) CreateServiceBrokerCallCount() int { + fake.createServiceBrokerMutex.RLock() + defer fake.createServiceBrokerMutex.RUnlock() + return len(fake.createServiceBrokerArgsForCall) +} + +func (fake *CFServiceBrokerRepository) CreateServiceBrokerCalls(stub func(context.Context, authorization.Info, repositories.CreateServiceBrokerMessage) (repositories.ServiceBrokerResource, error)) { + fake.createServiceBrokerMutex.Lock() + defer fake.createServiceBrokerMutex.Unlock() + fake.CreateServiceBrokerStub = stub +} + +func (fake *CFServiceBrokerRepository) CreateServiceBrokerArgsForCall(i int) (context.Context, authorization.Info, repositories.CreateServiceBrokerMessage) { + fake.createServiceBrokerMutex.RLock() + defer fake.createServiceBrokerMutex.RUnlock() + argsForCall := fake.createServiceBrokerArgsForCall[i] + return argsForCall.arg1, argsForCall.arg2, argsForCall.arg3 +} + +func (fake *CFServiceBrokerRepository) CreateServiceBrokerReturns(result1 repositories.ServiceBrokerResource, result2 error) { + fake.createServiceBrokerMutex.Lock() + defer fake.createServiceBrokerMutex.Unlock() + fake.CreateServiceBrokerStub = nil + fake.createServiceBrokerReturns = struct { + result1 repositories.ServiceBrokerResource + result2 error + }{result1, result2} +} + +func (fake *CFServiceBrokerRepository) CreateServiceBrokerReturnsOnCall(i int, result1 repositories.ServiceBrokerResource, result2 error) { + fake.createServiceBrokerMutex.Lock() + defer fake.createServiceBrokerMutex.Unlock() + fake.CreateServiceBrokerStub = nil + if fake.createServiceBrokerReturnsOnCall == nil { + fake.createServiceBrokerReturnsOnCall = make(map[int]struct { + result1 repositories.ServiceBrokerResource + result2 error + }) + } + fake.createServiceBrokerReturnsOnCall[i] = struct { + result1 repositories.ServiceBrokerResource + result2 error + }{result1, result2} +} + +func (fake *CFServiceBrokerRepository) Invocations() map[string][][]interface{} { + fake.invocationsMutex.RLock() + defer fake.invocationsMutex.RUnlock() + fake.createServiceBrokerMutex.RLock() + defer fake.createServiceBrokerMutex.RUnlock() + copiedInvocations := map[string][][]interface{}{} + for key, value := range fake.invocations { + copiedInvocations[key] = value + } + return copiedInvocations +} + +func (fake *CFServiceBrokerRepository) recordInvocation(key string, args []interface{}) { + fake.invocationsMutex.Lock() + defer fake.invocationsMutex.Unlock() + if fake.invocations == nil { + fake.invocations = map[string][][]interface{}{} + } + if fake.invocations[key] == nil { + fake.invocations[key] = [][]interface{}{} + } + fake.invocations[key] = append(fake.invocations[key], args) +} + +var _ handlers.CFServiceBrokerRepository = new(CFServiceBrokerRepository) diff --git a/api/handlers/fake/state_repository.go b/api/handlers/fake/state_repository.go new file mode 100644 index 000000000..dfd4712c5 --- /dev/null +++ b/api/handlers/fake/state_repository.go @@ -0,0 +1,123 @@ +// Code generated by counterfeiter. DO NOT EDIT. +package fake + +import ( + "context" + "sync" + + "code.cloudfoundry.org/korifi/api/authorization" + "code.cloudfoundry.org/korifi/api/handlers" + "code.cloudfoundry.org/korifi/model" +) + +type StateRepository struct { + GetStateStub func(context.Context, authorization.Info, string) (model.CFResourceState, error) + getStateMutex sync.RWMutex + getStateArgsForCall []struct { + arg1 context.Context + arg2 authorization.Info + arg3 string + } + getStateReturns struct { + result1 model.CFResourceState + result2 error + } + getStateReturnsOnCall map[int]struct { + result1 model.CFResourceState + result2 error + } + invocations map[string][][]interface{} + invocationsMutex sync.RWMutex +} + +func (fake *StateRepository) GetState(arg1 context.Context, arg2 authorization.Info, arg3 string) (model.CFResourceState, error) { + fake.getStateMutex.Lock() + ret, specificReturn := fake.getStateReturnsOnCall[len(fake.getStateArgsForCall)] + fake.getStateArgsForCall = append(fake.getStateArgsForCall, struct { + arg1 context.Context + arg2 authorization.Info + arg3 string + }{arg1, arg2, arg3}) + stub := fake.GetStateStub + fakeReturns := fake.getStateReturns + fake.recordInvocation("GetState", []interface{}{arg1, arg2, arg3}) + fake.getStateMutex.Unlock() + if stub != nil { + return stub(arg1, arg2, arg3) + } + if specificReturn { + return ret.result1, ret.result2 + } + return fakeReturns.result1, fakeReturns.result2 +} + +func (fake *StateRepository) GetStateCallCount() int { + fake.getStateMutex.RLock() + defer fake.getStateMutex.RUnlock() + return len(fake.getStateArgsForCall) +} + +func (fake *StateRepository) GetStateCalls(stub func(context.Context, authorization.Info, string) (model.CFResourceState, error)) { + fake.getStateMutex.Lock() + defer fake.getStateMutex.Unlock() + fake.GetStateStub = stub +} + +func (fake *StateRepository) GetStateArgsForCall(i int) (context.Context, authorization.Info, string) { + fake.getStateMutex.RLock() + defer fake.getStateMutex.RUnlock() + argsForCall := fake.getStateArgsForCall[i] + return argsForCall.arg1, argsForCall.arg2, argsForCall.arg3 +} + +func (fake *StateRepository) GetStateReturns(result1 model.CFResourceState, result2 error) { + fake.getStateMutex.Lock() + defer fake.getStateMutex.Unlock() + fake.GetStateStub = nil + fake.getStateReturns = struct { + result1 model.CFResourceState + result2 error + }{result1, result2} +} + +func (fake *StateRepository) GetStateReturnsOnCall(i int, result1 model.CFResourceState, result2 error) { + fake.getStateMutex.Lock() + defer fake.getStateMutex.Unlock() + fake.GetStateStub = nil + if fake.getStateReturnsOnCall == nil { + fake.getStateReturnsOnCall = make(map[int]struct { + result1 model.CFResourceState + result2 error + }) + } + fake.getStateReturnsOnCall[i] = struct { + result1 model.CFResourceState + result2 error + }{result1, result2} +} + +func (fake *StateRepository) Invocations() map[string][][]interface{} { + fake.invocationsMutex.RLock() + defer fake.invocationsMutex.RUnlock() + fake.getStateMutex.RLock() + defer fake.getStateMutex.RUnlock() + copiedInvocations := map[string][][]interface{}{} + for key, value := range fake.invocations { + copiedInvocations[key] = value + } + return copiedInvocations +} + +func (fake *StateRepository) recordInvocation(key string, args []interface{}) { + fake.invocationsMutex.Lock() + defer fake.invocationsMutex.Unlock() + if fake.invocations == nil { + fake.invocations = map[string][][]interface{}{} + } + if fake.invocations[key] == nil { + fake.invocations[key] = [][]interface{}{} + } + fake.invocations[key] = append(fake.invocations[key], args) +} + +var _ handlers.StateRepository = new(StateRepository) diff --git a/api/handlers/job.go b/api/handlers/job.go index f7a8c32fd..b1254993f 100644 --- a/api/handlers/job.go +++ b/api/handlers/job.go @@ -12,18 +12,20 @@ import ( apierrors "code.cloudfoundry.org/korifi/api/errors" "code.cloudfoundry.org/korifi/api/presenter" "code.cloudfoundry.org/korifi/api/routing" + "code.cloudfoundry.org/korifi/model" "code.cloudfoundry.org/korifi/tools/logger" ) const ( - JobPath = "/v3/jobs/{guid}" - syncSpaceJobType = "space.apply_manifest" - AppDeleteJobType = "app.delete" - OrgDeleteJobType = "org.delete" - RouteDeleteJobType = "route.delete" - SpaceDeleteJobType = "space.delete" - DomainDeleteJobType = "domain.delete" - RoleDeleteJobType = "role.delete" + JobPath = "/v3/jobs/{guid}" + syncSpaceJobType = "space.apply_manifest" + AppDeleteJobType = "app.delete" + OrgDeleteJobType = "org.delete" + RouteDeleteJobType = "route.delete" + SpaceDeleteJobType = "space.delete" + DomainDeleteJobType = "domain.delete" + RoleDeleteJobType = "role.delete" + ServiceBrokerCreateJobType = "service_broker.create" JobTimeoutDuration = 120.0 ) @@ -35,17 +37,29 @@ type DeletionRepository interface { GetDeletedAt(context.Context, authorization.Info, string) (*time.Time, error) } +//counterfeiter:generate -o fake -fake-name StateRepository . StateRepository +type StateRepository interface { + GetState(context.Context, authorization.Info, string) (model.CFResourceState, error) +} + type Job struct { - serverURL url.URL - repositories map[string]DeletionRepository - pollingInterval time.Duration + serverURL url.URL + deletionRepositories map[string]DeletionRepository + stateRepositories map[string]StateRepository + pollingInterval time.Duration } -func NewJob(serverURL url.URL, repositories map[string]DeletionRepository, pollingInterval time.Duration) *Job { +func NewJob( + serverURL url.URL, + deletionRepositories map[string]DeletionRepository, + stateRepositories map[string]StateRepository, + pollingInterval time.Duration, +) *Job { return &Job{ - serverURL: serverURL, - repositories: repositories, - pollingInterval: pollingInterval, + serverURL: serverURL, + deletionRepositories: deletionRepositories, + stateRepositories: stateRepositories, + pollingInterval: pollingInterval, } } @@ -63,25 +77,37 @@ func (h *Job) get(r *http.Request) (*routing.Response, error) { ) } - if job.Type == syncSpaceJobType { + switch job.Type { + case syncSpaceJobType: return routing.NewResponse(http.StatusOK).WithBody(presenter.ForManifestApplyJob(job, h.serverURL)), nil - } + default: + deletionRepository, ok := h.deletionRepositories[job.Type] + if ok { + jobResponse, err := h.handleDeleteJob(ctx, deletionRepository, job) + if err != nil { + return nil, err + } + + return routing.NewResponse(http.StatusOK).WithBody(jobResponse), nil + } + + stateRepository, ok := h.stateRepositories[job.Type] + if ok { + jobResponse, err := h.handleStateJob(ctx, stateRepository, job) + if err != nil { + return nil, err + } + + return routing.NewResponse(http.StatusOK).WithBody(jobResponse), nil + } - repository, ok := h.repositories[job.Type] - if !ok { return nil, apierrors.LogAndReturn( log, apierrors.NewNotFoundError(fmt.Errorf("invalid job type: %s", job.Type), JobResourceType), fmt.Sprintf("Invalid Job type: %s", job.Type), ) - } - jobResponse, err := h.handleDeleteJob(ctx, repository, job) - if err != nil { - return nil, err } - - return routing.NewResponse(http.StatusOK).WithBody(jobResponse), nil } func (h *Job) handleDeleteJob(ctx context.Context, repository DeletionRepository, job presenter.Job) (presenter.JobResponse, error) { @@ -135,6 +161,43 @@ func (h *Job) handleDeleteJob(ctx context.Context, repository DeletionRepository ), nil } +func (h *Job) handleStateJob(ctx context.Context, repository StateRepository, job presenter.Job) (presenter.JobResponse, error) { + ctx, log := logger.FromContext(ctx, "handleStateJob") + authInfo, _ := authorization.InfoFromContext(ctx) + state, err := repository.GetState(ctx, authInfo, job.ResourceGUID) + if err != nil { + if errors.As(err, &apierrors.ForbiddenError{}) { + return presenter.ForJob(job, + []presenter.JobResponseError{}, + presenter.StateComplete, + h.serverURL, + ), nil + } + return presenter.JobResponse{}, apierrors.LogAndReturn( + log, + err, + "failed to get "+job.ResourceType+" state from Kubernetes", + job.ResourceType+"GUID", job.ResourceGUID, + ) + } + + switch state.Status { + case model.CFResourceStatusReady: + return presenter.ForJob(job, + []presenter.JobResponseError{}, + presenter.StateComplete, + h.serverURL, + ), nil + + default: + return presenter.ForJob(job, + []presenter.JobResponseError{}, + presenter.StateProcessing, + h.serverURL, + ), nil + } +} + func (h *Job) retryGetDeletedAt(ctx context.Context, repository DeletionRepository, job presenter.Job) (*time.Time, error) { ctx, log := logger.FromContext(ctx, "retryGetDeletedAt") authInfo, _ := authorization.InfoFromContext(ctx) diff --git a/api/handlers/job_test.go b/api/handlers/job_test.go index 0a62c1cbc..0509c4c66 100644 --- a/api/handlers/job_test.go +++ b/api/handlers/job_test.go @@ -1,6 +1,7 @@ package handlers_test import ( + "errors" "fmt" "net/http" "time" @@ -8,6 +9,7 @@ import ( apierrors "code.cloudfoundry.org/korifi/api/errors" "code.cloudfoundry.org/korifi/api/handlers" "code.cloudfoundry.org/korifi/api/handlers/fake" + "code.cloudfoundry.org/korifi/model" . "code.cloudfoundry.org/korifi/tests/matchers" "code.cloudfoundry.org/korifi/tools" @@ -19,16 +21,18 @@ var _ = Describe("Job", func() { var ( handler *handlers.Job deletionRepos map[string]handlers.DeletionRepository + stateRepos map[string]handlers.StateRepository jobGUID string req *http.Request ) BeforeEach(func() { deletionRepos = map[string]handlers.DeletionRepository{} + stateRepos = map[string]handlers.StateRepository{} }) JustBeforeEach(func() { - handler = handlers.NewJob(*serverURL, deletionRepos, 0) + handler = handlers.NewJob(*serverURL, deletionRepos, stateRepos, 0) routerBuilder.LoadRoutes(handler) var err error @@ -56,7 +60,7 @@ var _ = Describe("Job", func() { }) }) - Describe("GET /v3/jobs/*", func() { + Describe("GET /v3/jobs/*delete*", func() { var deletionRepo *fake.DeletionRepository BeforeEach(func() { @@ -144,25 +148,91 @@ var _ = Describe("Job", func() { ))) }) }) + }) + + Describe("GET /v3/jobs/*state*", func() { + var stateRepo *fake.StateRepository + + BeforeEach(func() { + stateRepo = new(fake.StateRepository) + stateRepo.GetStateReturns(model.CFResourceState{}, nil) + stateRepos["testing.state"] = stateRepo - When("the job guid is invalid", func() { + jobGUID = "testing.state~my-resource-guid" + }) + + It("returns a processing status", func() { + Expect(stateRepo.GetStateCallCount()).To(Equal(1)) + _, actualAuthInfo, actualResourceGUID := stateRepo.GetStateArgsForCall(0) + Expect(actualAuthInfo).To(Equal(authInfo)) + Expect(actualResourceGUID).To(Equal("my-resource-guid")) + + Expect(rr).To(HaveHTTPStatus(http.StatusOK)) + Expect(rr).To(HaveHTTPBody(SatisfyAll( + MatchJSONPath("$.guid", jobGUID), + MatchJSONPath("$.links.self.href", defaultServerURL+"/v3/jobs/"+jobGUID), + MatchJSONPath("$.operation", "testing.state"), + MatchJSONPath("$.state", "PROCESSING"), + MatchJSONPath("$.errors", BeEmpty()), + ))) + }) + + When("the resource state is Ready", func() { BeforeEach(func() { - jobGUID = "job.operation;some-resource-guid" + stateRepo.GetStateReturns(model.CFResourceState{ + Status: model.CFResourceStatusReady, + }, nil) }) - It("returns an error", func() { - expectNotFoundError("Job") + It("returns a complete status", func() { + Expect(rr).To(HaveHTTPBody(SatisfyAll( + MatchJSONPath("$.state", "COMPLETE"), + MatchJSONPath("$.errors", BeEmpty()), + ))) }) }) - When("there is no deletion repository registered for the operation", func() { + When("the user does not have permission to see the resource", func() { + BeforeEach(func() { + stateRepo.GetStateReturns(model.CFResourceState{}, fmt.Errorf("wrapped err: %w", apierrors.NewForbiddenError(nil, "foo"))) + }) + + It("returns a complete status", func() { + Expect(rr).To(HaveHTTPBody(SatisfyAll( + MatchJSONPath("$.state", "COMPLETE"), + MatchJSONPath("$.errors", BeEmpty()), + ))) + }) + }) + + When("getting the state fails", func() { BeforeEach(func() { - deletionRepos = map[string]handlers.DeletionRepository{} + stateRepo.GetStateReturns(model.CFResourceState{}, errors.New("get-state-error")) }) It("returns an error", func() { - expectNotFoundError("Job") + expectUnknownError() }) }) }) + + When("the job type is unknown", func() { + BeforeEach(func() { + jobGUID = "unknown~guid" + }) + + It("returns an error", func() { + expectNotFoundError("Job") + }) + }) + + When("the job guid is invalid", func() { + BeforeEach(func() { + jobGUID = "job.operation;some-resource-guid" + }) + + It("returns an error", func() { + expectNotFoundError("Job") + }) + }) }) diff --git a/api/handlers/service_broker.go b/api/handlers/service_broker.go new file mode 100644 index 000000000..758ed4a90 --- /dev/null +++ b/api/handlers/service_broker.go @@ -0,0 +1,77 @@ +package handlers + +import ( + "context" + "net/http" + "net/url" + + "code.cloudfoundry.org/korifi/api/authorization" + apierrors "code.cloudfoundry.org/korifi/api/errors" + "code.cloudfoundry.org/korifi/api/payloads" + "code.cloudfoundry.org/korifi/api/presenter" + "code.cloudfoundry.org/korifi/api/repositories" + "code.cloudfoundry.org/korifi/api/routing" + "github.com/go-logr/logr" +) + +const ( + ServiceBrokersPath = "/v3/service_brokers" +) + +//counterfeiter:generate -o fake -fake-name CFServiceBrokerRepository . CFServiceBrokerRepository +type CFServiceBrokerRepository interface { + CreateServiceBroker(context.Context, authorization.Info, repositories.CreateServiceBrokerMessage) (repositories.ServiceBrokerResource, error) +} + +type ServiceBroker struct { + serverURL url.URL + serviceBrokerRepo CFServiceBrokerRepository + requestValidator RequestValidator + managedServicesEnabled bool +} + +func NewServiceBroker( + serverURL url.URL, + serviceBrokerRepo CFServiceBrokerRepository, + requestValidator RequestValidator, + managedServicesEnabled bool, +) *ServiceBroker { + return &ServiceBroker{ + serverURL: serverURL, + serviceBrokerRepo: serviceBrokerRepo, + requestValidator: requestValidator, + managedServicesEnabled: managedServicesEnabled, + } +} + +func (h *ServiceBroker) create(r *http.Request) (*routing.Response, error) { + if !h.managedServicesEnabled { + return nil, apierrors.NewUnprocessableEntityError(nil, "Managed services are not enabled") + } + + authInfo, _ := authorization.InfoFromContext(r.Context()) + logger := logr.FromContextOrDiscard(r.Context()).WithName("handlers.service-broker.create") + + payload := payloads.ServiceBrokerCreate{} + if err := h.requestValidator.DecodeAndValidateJSONPayload(r, &payload); err != nil { + return nil, apierrors.LogAndReturn(logger, err, "failed to decode payload") + } + + broker, err := h.serviceBrokerRepo.CreateServiceBroker(r.Context(), authInfo, payload.ToCreateServiceBrokerMessage()) + if err != nil { + return nil, apierrors.LogAndReturn(logger, err, "failed to create service broker") + } + + return routing.NewResponse(http.StatusAccepted). + WithHeader("Location", presenter.JobURLForRedirects(broker.GUID, presenter.ServiceBrokerCreateOperation, h.serverURL)), nil +} + +func (h *ServiceBroker) UnauthenticatedRoutes() []routing.Route { + return nil +} + +func (h *ServiceBroker) AuthenticatedRoutes() []routing.Route { + return []routing.Route{ + {Method: "POST", Pattern: ServiceBrokersPath, Handler: h.create}, + } +} diff --git a/api/handlers/service_broker_test.go b/api/handlers/service_broker_test.go new file mode 100644 index 000000000..9b86bcd87 --- /dev/null +++ b/api/handlers/service_broker_test.go @@ -0,0 +1,115 @@ +package handlers_test + +import ( + "errors" + "net/http" + "strings" + + "code.cloudfoundry.org/korifi/api/handlers" + "code.cloudfoundry.org/korifi/api/handlers/fake" + "code.cloudfoundry.org/korifi/api/payloads" + "code.cloudfoundry.org/korifi/api/repositories" + "code.cloudfoundry.org/korifi/model" + "code.cloudfoundry.org/korifi/model/services" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +var _ = Describe("ServiceBroker", func() { + var ( + serviceBrokerRepo *fake.CFServiceBrokerRepository + requestValidator *fake.RequestValidator + + req *http.Request + handler *handlers.ServiceBroker + ) + + BeforeEach(func() { + serviceBrokerRepo = new(fake.CFServiceBrokerRepository) + requestValidator = new(fake.RequestValidator) + handler = handlers.NewServiceBroker( + *serverURL, + serviceBrokerRepo, + requestValidator, + true, + ) + }) + + JustBeforeEach(func() { + routerBuilder.LoadRoutes(handler) + routerBuilder.Build().ServeHTTP(rr, req) + }) + + Describe("POST /v3/service_brokers", func() { + var payload payloads.ServiceBrokerCreate + + BeforeEach(func() { + payload = payloads.ServiceBrokerCreate{ + ServiceBroker: services.ServiceBroker{ + Name: "my-broker", + }, + Authentication: &payloads.BrokerAuthentication{}, + } + requestValidator.DecodeAndValidateJSONPayloadStub = decodeAndValidatePayloadStub(&payload) + + serviceBrokerRepo.CreateServiceBrokerReturns(repositories.ServiceBrokerResource{ + CFResource: model.CFResource{ + GUID: "service-broker-guid", + }, + }, nil) + var err error + req, err = http.NewRequestWithContext(ctx, "POST", "/v3/service_brokers", strings.NewReader("request-body")) + Expect(err).NotTo(HaveOccurred()) + }) + + It("creates a service broker", func() { + Expect(requestValidator.DecodeAndValidateJSONPayloadCallCount()).To(Equal(1)) + actualReq, _ := requestValidator.DecodeAndValidateJSONPayloadArgsForCall(0) + Expect(bodyString(actualReq)).To(Equal("request-body")) + + Expect(serviceBrokerRepo.CreateServiceBrokerCallCount()).To(Equal(1)) + _, actualAuthInfo, actualCreateMsg := serviceBrokerRepo.CreateServiceBrokerArgsForCall(0) + Expect(actualAuthInfo).To(Equal(authInfo)) + Expect(actualCreateMsg.Broker.Name).To(Equal("my-broker")) + + Expect(rr).To(HaveHTTPStatus(http.StatusAccepted)) + Expect(rr).To(HaveHTTPHeaderWithValue("Location", "https://api.example.org/v3/jobs/service_broker.create~service-broker-guid")) + }) + + When("the request body is invalid json", func() { + BeforeEach(func() { + requestValidator.DecodeAndValidateJSONPayloadReturns(errors.New("boom")) + }) + + It("returns an error", func() { + expectUnknownError() + }) + }) + + When("creating the service broker fails", func() { + BeforeEach(func() { + serviceBrokerRepo.CreateServiceBrokerReturns(repositories.ServiceBrokerResource{}, errors.New("create-service-broker-error")) + }) + + It("returns an error", func() { + expectUnknownError() + }) + }) + + When("managed services are not enabled", func() { + BeforeEach(func() { + handler = handlers.NewServiceBroker( + *serverURL, + serviceBrokerRepo, + requestValidator, + false, + ) + }) + + It("returns unprocessable entity error", func() { + expectUnprocessableEntityError("Managed services are not enabled") + }) + }) + }) +}) diff --git a/api/main.go b/api/main.go index 3fdefc2a4..d94ba632c 100644 --- a/api/main.go +++ b/api/main.go @@ -226,6 +226,7 @@ func main() { conditions.NewConditionAwaiter[*korifiv1alpha1.CFTask, korifiv1alpha1.CFTaskList](conditionTimeout), ) metricsRepo := repositories.NewMetricsRepo(userClientFactory) + serviceBrokerRepo := repositories.NewServiceBrokerRepo(userClientFactory, cfg.RootNamespace) processStats := actions.NewProcessStats(processRepo, appRepo, metricsRepo) manifest := actions.NewManifest( @@ -341,6 +342,9 @@ func main() { handlers.DomainDeleteJobType: domainRepo, handlers.RoleDeleteJobType: roleRepo, }, + map[string]handlers.StateRepository{ + handlers.ServiceBrokerCreateJobType: serviceBrokerRepo, + }, 500*time.Millisecond, ), handlers.NewLogCache( @@ -402,6 +406,12 @@ func main() { handlers.NewOAuth( *serverURL, ), + handlers.NewServiceBroker( + *serverURL, + serviceBrokerRepo, + requestValidator, + cfg.ExperimentalManagedServicesEnabled, + ), } for _, handler := range apiHandlers { routerBuilder.LoadRoutes(handler) diff --git a/api/payloads/service_broker.go b/api/payloads/service_broker.go new file mode 100644 index 000000000..9a87d8339 --- /dev/null +++ b/api/payloads/service_broker.go @@ -0,0 +1,42 @@ +package payloads + +import ( + "code.cloudfoundry.org/korifi/api/payloads/validation" + "code.cloudfoundry.org/korifi/api/repositories" + "code.cloudfoundry.org/korifi/model" + "code.cloudfoundry.org/korifi/model/services" + jellidation "github.com/jellydator/validation" +) + +type BrokerAuthentication struct { + Credentials services.BrokerCredentials `json:"credentials"` + Type string `json:"type"` +} + +func (a BrokerAuthentication) Validate() error { + return jellidation.ValidateStruct(&a, + jellidation.Field(&a.Type, validation.OneOf("basic")), + ) +} + +type ServiceBrokerCreate struct { + services.ServiceBroker + model.Metadata + Authentication *BrokerAuthentication `json:"authentication"` +} + +func (c ServiceBrokerCreate) Validate() error { + return jellidation.ValidateStruct(&c, + jellidation.Field(&c.Name, jellidation.Required), + jellidation.Field(&c.URL, jellidation.Required), + jellidation.Field(&c.Authentication, jellidation.Required), + ) +} + +func (c ServiceBrokerCreate) ToCreateServiceBrokerMessage() repositories.CreateServiceBrokerMessage { + return repositories.CreateServiceBrokerMessage{ + Broker: c.ServiceBroker, + Metadata: c.Metadata, + Credentials: c.Authentication.Credentials, + } +} diff --git a/api/payloads/service_broker_test.go b/api/payloads/service_broker_test.go new file mode 100644 index 000000000..b45a345a7 --- /dev/null +++ b/api/payloads/service_broker_test.go @@ -0,0 +1,113 @@ +package payloads_test + +import ( + "code.cloudfoundry.org/korifi/api/payloads" + "code.cloudfoundry.org/korifi/api/repositories" + "code.cloudfoundry.org/korifi/model" + "code.cloudfoundry.org/korifi/model/services" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + . "github.com/onsi/gomega/gstruct" +) + +var _ = Describe("ServiceBrokerCreate", func() { + var ( + createPayload payloads.ServiceBrokerCreate + serviceBrokerCreate *payloads.ServiceBrokerCreate + validatorErr error + ) + + BeforeEach(func() { + serviceBrokerCreate = new(payloads.ServiceBrokerCreate) + createPayload = payloads.ServiceBrokerCreate{ + Metadata: model.Metadata{ + Labels: map[string]string{ + "label": "label-value", + }, + Annotations: map[string]string{ + "annotation": "annotation-value", + }, + }, + ServiceBroker: services.ServiceBroker{ + Name: "my-broker", + URL: "https://my.broker.com", + }, + Authentication: &payloads.BrokerAuthentication{ + Credentials: services.BrokerCredentials{ + Username: "broker-user", + Password: "broker-password", + }, + Type: "basic", + }, + } + }) + + JustBeforeEach(func() { + validatorErr = validator.DecodeAndValidateJSONPayload(createJSONRequest(createPayload), serviceBrokerCreate) + }) + + It("succeeds", func() { + Expect(validatorErr).NotTo(HaveOccurred()) + Expect(serviceBrokerCreate).To(PointTo(Equal(createPayload))) + }) + + When("name is not set", func() { + BeforeEach(func() { + createPayload.Name = "" + }) + + It("returns an appropriate error", func() { + expectUnprocessableEntityError(validatorErr, "name cannot be blank") + }) + }) + + When("url is not set", func() { + BeforeEach(func() { + createPayload.URL = "" + }) + + It("returns an appropriate error", func() { + expectUnprocessableEntityError(validatorErr, "url cannot be blank") + }) + }) + + When("authentication is not set", func() { + BeforeEach(func() { + createPayload.Authentication = nil + }) + + It("returns an appropriate error", func() { + expectUnprocessableEntityError(validatorErr, "authentication cannot be blank") + }) + }) + + When("authentication type is invalid", func() { + BeforeEach(func() { + createPayload.Authentication.Type = "invalid-auth" + }) + + It("returns an appropriate error", func() { + expectUnprocessableEntityError(validatorErr, "type value must be one of: basic") + }) + }) + + Describe("ToServiceBrokerCreateMessage()", func() { + It("converts to repo message correctly", func() { + msg := serviceBrokerCreate.ToCreateServiceBrokerMessage() + Expect(msg).To(Equal(repositories.CreateServiceBrokerMessage{ + Metadata: model.Metadata{ + Labels: map[string]string{"label": "label-value"}, + Annotations: map[string]string{"annotation": "annotation-value"}, + }, + Broker: services.ServiceBroker{ + Name: "my-broker", + URL: "https://my.broker.com", + }, + Credentials: services.BrokerCredentials{ + Username: "broker-user", + Password: "broker-password", + }, + })) + }) + }) +}) diff --git a/api/presenter/job.go b/api/presenter/job.go index 354056e14..876b99f54 100644 --- a/api/presenter/job.go +++ b/api/presenter/job.go @@ -16,13 +16,14 @@ const ( StateFailed = "FAILED" StateProcessing = "PROCESSING" - AppDeleteOperation = "app.delete" - OrgDeleteOperation = "org.delete" - RouteDeleteOperation = "route.delete" - SpaceApplyManifestOperation = "space.apply_manifest" - SpaceDeleteOperation = "space.delete" - DomainDeleteOperation = "domain.delete" - RoleDeleteOperation = "role.delete" + AppDeleteOperation = "app.delete" + OrgDeleteOperation = "org.delete" + RouteDeleteOperation = "route.delete" + SpaceApplyManifestOperation = "space.apply_manifest" + SpaceDeleteOperation = "space.delete" + DomainDeleteOperation = "domain.delete" + RoleDeleteOperation = "role.delete" + ServiceBrokerCreateOperation = "service_broker.create" ) var ( diff --git a/api/repositories/service_broker_repository.go b/api/repositories/service_broker_repository.go new file mode 100644 index 000000000..498070b20 --- /dev/null +++ b/api/repositories/service_broker_repository.go @@ -0,0 +1,134 @@ +package repositories + +import ( + "context" + "fmt" + + "code.cloudfoundry.org/korifi/api/authorization" + apierrors "code.cloudfoundry.org/korifi/api/errors" + korifiv1alpha1 "code.cloudfoundry.org/korifi/controllers/api/v1alpha1" + "code.cloudfoundry.org/korifi/controllers/controllers/services/credentials" + "code.cloudfoundry.org/korifi/model" + "code.cloudfoundry.org/korifi/model/services" + "github.com/google/uuid" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes/scheme" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" +) + +const ServiceBrokerResourceType = "Service Broker" + +type CreateServiceBrokerMessage struct { + Metadata model.Metadata + Broker services.ServiceBroker + Credentials services.BrokerCredentials +} + +type ServiceBrokerRepo struct { + userClientFactory authorization.UserK8sClientFactory + rootNamespace string +} + +type ServiceBrokerResource struct { + services.ServiceBroker + model.Metadata + model.CFResource +} + +func NewServiceBrokerRepo( + userClientFactory authorization.UserK8sClientFactory, + rootNamespace string, +) *ServiceBrokerRepo { + return &ServiceBrokerRepo{ + userClientFactory: userClientFactory, + rootNamespace: rootNamespace, + } +} + +func (r *ServiceBrokerRepo) CreateServiceBroker(ctx context.Context, authInfo authorization.Info, message CreateServiceBrokerMessage) (ServiceBrokerResource, error) { + userClient, err := r.userClientFactory.BuildClient(authInfo) + if err != nil { + return ServiceBrokerResource{}, fmt.Errorf("failed to build user client: %w", err) + } + + credsSecretData, err := credentials.ToCredentialsSecretData(message.Credentials) + if err != nil { + return ServiceBrokerResource{}, fmt.Errorf("failed to create credentials secret data: %w", err) + } + + credentialsSecretName := uuid.NewString() + cfServiceBroker := &korifiv1alpha1.CFServiceBroker{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: r.rootNamespace, + Name: uuid.NewString(), + Labels: message.Metadata.Labels, + Annotations: message.Metadata.Annotations, + }, + Spec: korifiv1alpha1.CFServiceBrokerSpec{ + ServiceBroker: message.Broker, + Credentials: corev1.LocalObjectReference{ + Name: credentialsSecretName, + }, + }, + } + if err = userClient.Create(ctx, cfServiceBroker); err != nil { + return ServiceBrokerResource{}, apierrors.FromK8sError(err, ServiceBrokerResourceType) + } + + credentialsSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: r.rootNamespace, + Name: credentialsSecretName, + }, + Data: credsSecretData, + } + err = controllerutil.SetOwnerReference(cfServiceBroker, credentialsSecret, scheme.Scheme) + if err != nil { + return ServiceBrokerResource{}, apierrors.FromK8sError(err, ServiceBrokerResourceType) + } + + if err = userClient.Create(ctx, credentialsSecret); err != nil { + return ServiceBrokerResource{}, apierrors.FromK8sError(err, ServiceBrokerResourceType) + } + + return ServiceBrokerResource{ + ServiceBroker: cfServiceBroker.Spec.ServiceBroker, + Metadata: model.Metadata{ + Labels: cfServiceBroker.Labels, + Annotations: cfServiceBroker.Annotations, + }, + CFResource: model.CFResource{ + GUID: cfServiceBroker.Name, + CreatedAt: cfServiceBroker.CreationTimestamp.Time, + }, + }, nil +} + +func (r *ServiceBrokerRepo) GetState(ctx context.Context, authInfo authorization.Info, brokerGUID string) (model.CFResourceState, error) { + userClient, err := r.userClientFactory.BuildClient(authInfo) + if err != nil { + return model.CFResourceState{}, fmt.Errorf("failed to build user client: %w", err) + } + + cfServiceBroker := &korifiv1alpha1.CFServiceBroker{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: r.rootNamespace, + Name: brokerGUID, + }, + } + + if err = userClient.Get(ctx, client.ObjectKeyFromObject(cfServiceBroker), cfServiceBroker); err != nil { + return model.CFResourceState{}, apierrors.FromK8sError(err, ServiceBrokerResourceType) + } + + if meta.IsStatusConditionTrue(cfServiceBroker.Status.Conditions, korifiv1alpha1.StatusConditionReady) { + return model.CFResourceState{ + Status: model.CFResourceStatusReady, + }, nil + } + + return model.CFResourceState{}, nil +} diff --git a/api/repositories/service_broker_repository_test.go b/api/repositories/service_broker_repository_test.go new file mode 100644 index 000000000..6a03eefa6 --- /dev/null +++ b/api/repositories/service_broker_repository_test.go @@ -0,0 +1,229 @@ +package repositories_test + +import ( + apierrors "code.cloudfoundry.org/korifi/api/errors" + "code.cloudfoundry.org/korifi/api/repositories" + korifiv1alpha1 "code.cloudfoundry.org/korifi/controllers/api/v1alpha1" + "code.cloudfoundry.org/korifi/model" + "code.cloudfoundry.org/korifi/model/services" + "code.cloudfoundry.org/korifi/tools/k8s" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/google/uuid" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + . "github.com/onsi/gomega/gstruct" +) + +var _ = Describe("ServiceBrokerRepo", func() { + var repo *repositories.ServiceBrokerRepo + + BeforeEach(func() { + repo = repositories.NewServiceBrokerRepo(userClientFactory, rootNamespace) + }) + + Describe("Create", func() { + var ( + createMsg repositories.CreateServiceBrokerMessage + broker repositories.ServiceBrokerResource + createErr error + ) + + BeforeEach(func() { + createMsg = repositories.CreateServiceBrokerMessage{ + Metadata: model.Metadata{ + Labels: map[string]string{ + "label": "label-value", + }, + Annotations: map[string]string{ + "annotation": "annotation-value", + }, + }, + Broker: services.ServiceBroker{ + Name: "my-broker", + URL: "https://my.broker.com", + }, + Credentials: services.BrokerCredentials{ + Username: "broker-user", + Password: "broker-password", + }, + } + }) + + JustBeforeEach(func() { + broker, createErr = repo.CreateServiceBroker(ctx, authInfo, createMsg) + }) + + It("returns a forbidden error", func() { + Expect(createErr).To(SatisfyAll( + BeAssignableToTypeOf(apierrors.ForbiddenError{}), + MatchError(ContainSubstring("cfservicebrokers.korifi.cloudfoundry.org is forbidden")), + )) + }) + + It("does not create a credentials secret", func() { + secrets := &corev1.SecretList{} + Expect(k8sClient.List(ctx, secrets, client.InNamespace(rootNamespace))).To(Succeed()) + Expect(secrets.Items).To(BeEmpty()) + }) + + When("the user can create CFServiceBrokers", func() { + BeforeEach(func() { + createRoleBinding(ctx, userName, adminRole.Name, rootNamespace) + }) + + JustBeforeEach(func() { + Expect(createErr).NotTo(HaveOccurred()) + }) + + It("returns a ServiceBrokerResource", func() { + Expect(broker.ServiceBroker).To(Equal(services.ServiceBroker{ + Name: "my-broker", + URL: "https://my.broker.com", + })) + Expect(broker.Metadata).To(Equal(model.Metadata{ + Labels: map[string]string{ + "label": "label-value", + }, + Annotations: map[string]string{ + "annotation": "annotation-value", + }, + })) + Expect(broker.CFResource.GUID).NotTo(BeEmpty()) + Expect(broker.CFResource.CreatedAt).NotTo(BeZero()) + }) + + It("creates a CFServiceBroker resource in Kubernetes", func() { + cfServiceBroker := &korifiv1alpha1.CFServiceBroker{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: rootNamespace, + Name: broker.GUID, + }, + } + Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(cfServiceBroker), cfServiceBroker)).To(Succeed()) + Expect(cfServiceBroker.Labels).To(HaveKeyWithValue("label", "label-value")) + Expect(cfServiceBroker.Annotations).To(HaveKeyWithValue("annotation", "annotation-value")) + Expect(cfServiceBroker.Spec.Name).To(Equal("my-broker")) + Expect(cfServiceBroker.Spec.URL).To(Equal("https://my.broker.com")) + Expect(cfServiceBroker.Spec.Credentials.Name).NotTo(BeEmpty()) + }) + + It("stores broker credentials in a k8s secret", func() { + cfServiceBroker := &korifiv1alpha1.CFServiceBroker{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: rootNamespace, + Name: broker.GUID, + }, + } + Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(cfServiceBroker), cfServiceBroker)).To(Succeed()) + + Expect(cfServiceBroker.Spec.Credentials.Name).NotTo(BeEmpty()) + credentialsSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: rootNamespace, + Name: cfServiceBroker.Spec.Credentials.Name, + }, + } + Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(credentialsSecret), credentialsSecret)).To(Succeed()) + Expect(credentialsSecret.Data).To(SatisfyAll( + HaveLen(1), + HaveKeyWithValue(korifiv1alpha1.CredentialsSecretKey, + MatchJSON(`{"username" : "broker-user", "password": "broker-password"}`), + ), + )) + Expect(credentialsSecret.OwnerReferences).To(ConsistOf(MatchFields(IgnoreExtras, Fields{ + "Kind": Equal("CFServiceBroker"), + "Name": Equal(cfServiceBroker.Name), + }))) + }) + }) + }) + + Describe("GetState", func() { + var ( + brokerGUID string + cfServiceBroker *korifiv1alpha1.CFServiceBroker + state model.CFResourceState + getStateErr error + ) + + BeforeEach(func() { + brokerGUID = uuid.NewString() + cfServiceBroker = &korifiv1alpha1.CFServiceBroker{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: rootNamespace, + Name: brokerGUID, + }, + } + Expect(k8sClient.Create(ctx, cfServiceBroker)).To(Succeed()) + }) + + JustBeforeEach(func() { + state, getStateErr = repo.GetState(ctx, authInfo, brokerGUID) + }) + + It("returns a forbidden error", func() { + Expect(getStateErr).To(BeAssignableToTypeOf(apierrors.ForbiddenError{})) + }) + + When("the user can get CFServiceBrokers", func() { + BeforeEach(func() { + createRoleBinding(ctx, userName, adminRole.Name, rootNamespace) + }) + + JustBeforeEach(func() { + Expect(getStateErr).NotTo(HaveOccurred()) + }) + + It("returns unknown state", func() { + Expect(getStateErr).NotTo(HaveOccurred()) + Expect(state).To(Equal(model.CFResourceState{ + Status: model.CFResourceStatusUnknown, + })) + }) + + When("the broker is ready", func() { + BeforeEach(func() { + Expect(k8s.Patch(ctx, k8sClient, cfServiceBroker, func() { + meta.SetStatusCondition(&cfServiceBroker.Status.Conditions, metav1.Condition{ + Type: korifiv1alpha1.StatusConditionReady, + Status: metav1.ConditionTrue, + Message: "Ready", + Reason: "Ready", + }) + })).To(Succeed()) + }) + + It("returns ready state", func() { + Expect(getStateErr).NotTo(HaveOccurred()) + Expect(state).To(Equal(model.CFResourceState{ + Status: model.CFResourceStatusReady, + })) + }) + }) + + When("the broker is not ready", func() { + BeforeEach(func() { + Expect(k8s.Patch(ctx, k8sClient, cfServiceBroker, func() { + meta.SetStatusCondition(&cfServiceBroker.Status.Conditions, metav1.Condition{ + Type: korifiv1alpha1.StatusConditionReady, + Status: metav1.ConditionFalse, + Message: "NotReady", + Reason: "NotReady", + }) + })).To(Succeed()) + }) + + It("returns unknown state", func() { + Expect(getStateErr).NotTo(HaveOccurred()) + Expect(state).To(Equal(model.CFResourceState{ + Status: model.CFResourceStatusUnknown, + })) + }) + }) + }) + }) +}) diff --git a/api/repositories/service_instance_repository.go b/api/repositories/service_instance_repository.go index 89dd3ee4e..006b916fa 100644 --- a/api/repositories/service_instance_repository.go +++ b/api/repositories/service_instance_repository.go @@ -2,7 +2,6 @@ package repositories import ( "context" - "encoding/json" "errors" "fmt" "time" @@ -10,6 +9,7 @@ import ( "code.cloudfoundry.org/korifi/api/authorization" apierrors "code.cloudfoundry.org/korifi/api/errors" korifiv1alpha1 "code.cloudfoundry.org/korifi/controllers/api/v1alpha1" + "code.cloudfoundry.org/korifi/controllers/controllers/services/credentials" "code.cloudfoundry.org/korifi/tools/k8s" "github.com/google/uuid" @@ -201,7 +201,7 @@ func (r *ServiceInstanceRepo) createCredentialsSecret( ctx context.Context, userClient client.Client, cfServiceInstance *korifiv1alpha1.CFServiceInstance, - credentials map[string]any, + creds map[string]any, ) error { credentialsSecret := &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ @@ -217,7 +217,7 @@ func (r *ServiceInstanceRepo) createCredentialsSecret( credentialsSecret.Labels[CFServiceInstanceGUIDLabel] = cfServiceInstance.Name var err error - credentialsSecret.Data, err = toSecretData(credentials) + credentialsSecret.Data, err = credentials.ToCredentialsSecretData(creds) if err != nil { return errors.New("failed to marshal credentials for service instance") } @@ -227,18 +227,6 @@ func (r *ServiceInstanceRepo) createCredentialsSecret( return err } -func toSecretData(credentials map[string]any) (map[string][]byte, error) { - var credentialBytes []byte - credentialBytes, err := json.Marshal(credentials) - if err != nil { - return nil, errors.New("failed to marshal credentials for service instance") - } - - return map[string][]byte{ - korifiv1alpha1.CredentialsSecretKey: credentialBytes, - }, nil -} - // nolint:dupl func (r *ServiceInstanceRepo) ListServiceInstances(ctx context.Context, authInfo authorization.Info, message ListServiceInstanceMessage) ([]ServiceInstanceRecord, error) { nsList, err := r.namespacePermissions.GetAuthorizedSpaceNamespaces(ctx, authInfo) diff --git a/controllers/Dockerfile b/controllers/Dockerfile index b4f6dfbe9..8135e62be 100644 --- a/controllers/Dockerfile +++ b/controllers/Dockerfile @@ -10,6 +10,7 @@ COPY go.mod go.sum ./ RUN --mount=type=cache,target=/go/pkg/mod \ go mod download +COPY model model COPY controllers/api controllers/api COPY controllers/config/config.go controllers/config/config.go COPY controllers/controllers controllers/controllers diff --git a/controllers/api/v1alpha1/cfservicebroker_types.go b/controllers/api/v1alpha1/cfservicebroker_types.go new file mode 100644 index 000000000..2f3d3d019 --- /dev/null +++ b/controllers/api/v1alpha1/cfservicebroker_types.go @@ -0,0 +1,63 @@ +package v1alpha1 + +import ( + "strings" + + "code.cloudfoundry.org/korifi/model/services" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +const ( + UsernameCredentialsKey = "username" + PasswordCredentialsKey = "password" +) + +type CFServiceBrokerSpec struct { + services.ServiceBroker `json:",inline"` + Credentials corev1.LocalObjectReference `json:"credentials"` +} + +type CFServiceBrokerStatus struct { + //+kubebuilder:validation:Optional + Conditions []metav1.Condition `json:"conditions,omitempty"` + + // ObservedGeneration captures the latest generation of the CFServiceBroker that has been reconciled + ObservedGeneration int64 `json:"observedGeneration,omitempty"` + + // ObservedGeneration captures the latest version of the spec.Credentials.Name secret that has been reconciled + // This will ensure that interested contollers are notified on broker credentials change + //+kubebuilder:validation:Optional + CredentialsObservedVersion string `json:"credentialsObservedVersion,omitempty"` +} + +// +kubebuilder:object:root=true +// +kubebuilder:subresource:status +// +kubebuilder:printcolumn:name="Broker Name",type=string,JSONPath=`.spec.name` +// +kubebuilder:printcolumn:name="Age",type="date",JSONPath=`.metadata.creationTimestamp` +type CFServiceBroker struct { + metav1.TypeMeta `json:",inline"` + metav1.ObjectMeta `json:"metadata,omitempty"` + + Spec CFServiceBrokerSpec `json:"spec,omitempty"` + Status CFServiceBrokerStatus `json:"status,omitempty"` +} + +func (b CFServiceBroker) UniqueName() string { + return strings.ToLower(b.Spec.Name) +} + +func (b CFServiceBroker) UniqueValidationErrorMessage() string { + return "Name must be unique" +} + +// +kubebuilder:object:root=true +type CFServiceBrokerList struct { + metav1.TypeMeta `json:",inline"` + metav1.ListMeta `json:"metadata,omitempty"` + Items []CFServiceBroker `json:"items"` +} + +func init() { + SchemeBuilder.Register(&CFServiceBroker{}, &CFServiceBrokerList{}) +} diff --git a/controllers/api/v1alpha1/cfserviceinstance_types.go b/controllers/api/v1alpha1/cfserviceinstance_types.go index 840f3b8c2..af5872a45 100644 --- a/controllers/api/v1alpha1/cfserviceinstance_types.go +++ b/controllers/api/v1alpha1/cfserviceinstance_types.go @@ -24,8 +24,7 @@ import ( ) const ( - UserProvidedType = "user-provided" - CredentialsSecretKey = "credentials" + UserProvidedType = "user-provided" ) // CFServiceInstanceSpec defines the desired state of CFServiceInstance diff --git a/controllers/api/v1alpha1/shared_types.go b/controllers/api/v1alpha1/shared_types.go index f3ca8ec33..95c4272db 100644 --- a/controllers/api/v1alpha1/shared_types.go +++ b/controllers/api/v1alpha1/shared_types.go @@ -24,6 +24,8 @@ const ( PropagateServiceAccountAnnotation = "cloudfoundry.org/propagate-service-account" PropagateDeletionAnnotation = "cloudfoundry.org/propagate-deletion" PropagatedFromLabel = "cloudfoundry.org/propagated-from" + + CredentialsSecretKey = "credentials" ) type Lifecycle struct { diff --git a/controllers/api/v1alpha1/zz_generated.deepcopy.go b/controllers/api/v1alpha1/zz_generated.deepcopy.go index 5cb4db4c4..832426b1c 100644 --- a/controllers/api/v1alpha1/zz_generated.deepcopy.go +++ b/controllers/api/v1alpha1/zz_generated.deepcopy.go @@ -1281,6 +1281,104 @@ func (in *CFServiceBindingStatus) DeepCopy() *CFServiceBindingStatus { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *CFServiceBroker) DeepCopyInto(out *CFServiceBroker) { + *out = *in + out.TypeMeta = in.TypeMeta + in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) + out.Spec = in.Spec + in.Status.DeepCopyInto(&out.Status) +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new CFServiceBroker. +func (in *CFServiceBroker) DeepCopy() *CFServiceBroker { + if in == nil { + return nil + } + out := new(CFServiceBroker) + in.DeepCopyInto(out) + return out +} + +// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. +func (in *CFServiceBroker) DeepCopyObject() runtime.Object { + if c := in.DeepCopy(); c != nil { + return c + } + return nil +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *CFServiceBrokerList) DeepCopyInto(out *CFServiceBrokerList) { + *out = *in + out.TypeMeta = in.TypeMeta + in.ListMeta.DeepCopyInto(&out.ListMeta) + if in.Items != nil { + in, out := &in.Items, &out.Items + *out = make([]CFServiceBroker, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new CFServiceBrokerList. +func (in *CFServiceBrokerList) DeepCopy() *CFServiceBrokerList { + if in == nil { + return nil + } + out := new(CFServiceBrokerList) + in.DeepCopyInto(out) + return out +} + +// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. +func (in *CFServiceBrokerList) DeepCopyObject() runtime.Object { + if c := in.DeepCopy(); c != nil { + return c + } + return nil +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *CFServiceBrokerSpec) DeepCopyInto(out *CFServiceBrokerSpec) { + *out = *in + out.ServiceBroker = in.ServiceBroker + out.Credentials = in.Credentials +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new CFServiceBrokerSpec. +func (in *CFServiceBrokerSpec) DeepCopy() *CFServiceBrokerSpec { + if in == nil { + return nil + } + out := new(CFServiceBrokerSpec) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *CFServiceBrokerStatus) DeepCopyInto(out *CFServiceBrokerStatus) { + *out = *in + if in.Conditions != nil { + in, out := &in.Conditions, &out.Conditions + *out = make([]metav1.Condition, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new CFServiceBrokerStatus. +func (in *CFServiceBrokerStatus) DeepCopy() *CFServiceBrokerStatus { + if in == nil { + return nil + } + out := new(CFServiceBrokerStatus) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *CFServiceInstance) DeepCopyInto(out *CFServiceInstance) { *out = *in diff --git a/controllers/controllers/services/brokers/controller.go b/controllers/controllers/services/brokers/controller.go new file mode 100644 index 000000000..b79c108c3 --- /dev/null +++ b/controllers/controllers/services/brokers/controller.go @@ -0,0 +1,146 @@ +/* +Copyright 2021. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package brokers + +import ( + "context" + "encoding/json" + "fmt" + "time" + + korifiv1alpha1 "code.cloudfoundry.org/korifi/controllers/api/v1alpha1" + "code.cloudfoundry.org/korifi/controllers/controllers/shared" + "code.cloudfoundry.org/korifi/tools/k8s" + + "github.com/go-logr/logr" + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/builder" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/handler" + "sigs.k8s.io/controller-runtime/pkg/reconcile" +) + +type Reconciler struct { + k8sClient client.Client + scheme *runtime.Scheme + log logr.Logger +} + +func NewReconciler( + client client.Client, + scheme *runtime.Scheme, + log logr.Logger, +) *k8s.PatchingReconciler[korifiv1alpha1.CFServiceBroker, *korifiv1alpha1.CFServiceBroker] { + serviceInstanceReconciler := Reconciler{k8sClient: client, scheme: scheme, log: log} + return k8s.NewPatchingReconciler(log, client, &serviceInstanceReconciler) +} + +func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) *builder.Builder { + return ctrl.NewControllerManagedBy(mgr). + For(&korifiv1alpha1.CFServiceBroker{}). + Watches( + &corev1.Secret{}, + handler.EnqueueRequestsFromMapFunc(r.secretToServiceBroker), + ) +} + +func (r *Reconciler) secretToServiceBroker(ctx context.Context, o client.Object) []reconcile.Request { + serviceBrokers := korifiv1alpha1.CFServiceBrokerList{} + if err := r.k8sClient.List(ctx, &serviceBrokers, + client.InNamespace(o.GetNamespace()), + client.MatchingFields{ + shared.IndexServiceBrokerCredentialsSecretName: o.GetName(), + }); err != nil { + return []reconcile.Request{} + } + + requests := []reconcile.Request{} + for _, sb := range serviceBrokers.Items { + requests = append(requests, reconcile.Request{ + NamespacedName: types.NamespacedName{ + Name: sb.Name, + Namespace: sb.Namespace, + }, + }) + } + + return requests +} + +//+kubebuilder:rbac:groups=korifi.cloudfoundry.org,resources=cfservicebrokers,verbs=get;list;watch;create;update;patch;delete +//+kubebuilder:rbac:groups=korifi.cloudfoundry.org,resources=cfservicebrokers/status,verbs=get;update;patch + +func (r *Reconciler) ReconcileResource(ctx context.Context, cfServiceBroker *korifiv1alpha1.CFServiceBroker) (ctrl.Result, error) { + log := logr.FromContextOrDiscard(ctx) + + cfServiceBroker.Status.ObservedGeneration = cfServiceBroker.Generation + log.V(1).Info("set observed generation", "generation", cfServiceBroker.Status.ObservedGeneration) + + var err error + readyConditionBuilder := k8s.NewReadyConditionBuilder(cfServiceBroker) + defer func() { + meta.SetStatusCondition(&cfServiceBroker.Status.Conditions, readyConditionBuilder.WithError(err).Build()) + }() + + credentialsSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: cfServiceBroker.Namespace, + Name: cfServiceBroker.Spec.Credentials.Name, + }, + } + err = r.k8sClient.Get(ctx, client.ObjectKeyFromObject(credentialsSecret), credentialsSecret) + if err != nil { + readyConditionBuilder.WithReason("CredentialsSecretNotAvailable") + if apierrors.IsNotFound(err) { + return ctrl.Result{RequeueAfter: 2 * time.Second}, nil + } + return ctrl.Result{}, err + } + + if err = r.validateCredentials(credentialsSecret); err != nil { + readyConditionBuilder.WithReason("SecretInvalid") + return ctrl.Result{}, err + } + + log.V(1).Info("credentials secret", "name", credentialsSecret.Name, "version", credentialsSecret.ResourceVersion) + cfServiceBroker.Status.CredentialsObservedVersion = credentialsSecret.ResourceVersion + + readyConditionBuilder.Ready() + return ctrl.Result{}, nil +} + +func (r *Reconciler) validateCredentials(credentialsSecret *corev1.Secret) error { + creds := map[string]any{} + err := json.Unmarshal(credentialsSecret.Data[korifiv1alpha1.CredentialsSecretKey], &creds) + if err != nil { + return fmt.Errorf("invalid credentials secret %q: %w", credentialsSecret.Name, err) + } + + for _, k := range []string{korifiv1alpha1.UsernameCredentialsKey, korifiv1alpha1.PasswordCredentialsKey} { + if _, ok := creds[k]; !ok { + return fmt.Errorf("broker credentials secret %q does not specify %q", credentialsSecret.Name, k) + } + } + + return nil +} diff --git a/controllers/controllers/services/brokers/controller_test.go b/controllers/controllers/services/brokers/controller_test.go new file mode 100644 index 000000000..764ad54e7 --- /dev/null +++ b/controllers/controllers/services/brokers/controller_test.go @@ -0,0 +1,195 @@ +package brokers_test + +import ( + "github.com/google/uuid" + "sigs.k8s.io/controller-runtime/pkg/client" + + korifiv1alpha1 "code.cloudfoundry.org/korifi/controllers/api/v1alpha1" + . "code.cloudfoundry.org/korifi/tests/matchers" + "code.cloudfoundry.org/korifi/tools/k8s" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +var _ = Describe("CFServiceBroker", func() { + var ( + testNamespace string + broker *korifiv1alpha1.CFServiceBroker + ) + + BeforeEach(func() { + testNamespace = uuid.NewString() + Expect(adminClient.Create(ctx, &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: testNamespace, + }, + })).To(Succeed()) + + broker = &korifiv1alpha1.CFServiceBroker{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: testNamespace, + Name: uuid.NewString(), + }, + } + Expect(adminClient.Create(ctx, broker)).To(Succeed()) + }) + + It("sets the ObservedGeneration status field", func() { + Eventually(func(g Gomega) { + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(broker), broker)).To(Succeed()) + g.Expect(broker.Status.ObservedGeneration).To(Equal(broker.Generation)) + }).Should(Succeed()) + }) + + It("sets the Ready condition to false", func() { + Eventually(func(g Gomega) { + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(broker), broker)).To(Succeed()) + g.Expect(broker.Status.Conditions).To(ContainElement(SatisfyAll( + HasType(Equal(korifiv1alpha1.StatusConditionReady)), + HasStatus(Equal(metav1.ConditionFalse)), + HasReason(Equal("CredentialsSecretNotAvailable")), + ))) + }).Should(Succeed()) + }) + + Describe("credentials secret", func() { + var credentialsSecret *corev1.Secret + + BeforeEach(func() { + credentialsSecret = &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: uuid.NewString(), + Namespace: testNamespace, + }, + Data: map[string][]byte{ + korifiv1alpha1.CredentialsSecretKey: []byte(`{"username": "broker-user", "password": "broker-password"}`), + }, + } + Expect(adminClient.Create(ctx, credentialsSecret)).To(Succeed()) + Expect(k8s.PatchResource(ctx, adminClient, broker, func() { + broker.Spec.Credentials.Name = credentialsSecret.Name + })).To(Succeed()) + }) + + It("sets the Ready condition to true", func() { + Eventually(func(g Gomega) { + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(broker), broker)).To(Succeed()) + g.Expect(broker.Status.Conditions).To(ContainElement(SatisfyAll( + HasType(Equal(korifiv1alpha1.StatusConditionReady)), + HasStatus(Equal(metav1.ConditionTrue)), + ))) + }).Should(Succeed()) + }) + + It("sets the credentials secret observed version", func() { + Eventually(func(g Gomega) { + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(broker), broker)).To(Succeed()) + g.Expect(broker.Status.CredentialsObservedVersion).NotTo(BeEmpty()) + }).Should(Succeed()) + }) + + When("the credentials secret data does not have the credentials key", func() { + BeforeEach(func() { + Expect(k8s.PatchResource(ctx, adminClient, credentialsSecret, func() { + credentialsSecret.Data = map[string][]byte{} + })).To(Succeed()) + }) + + It("sets the ready condition to false", func() { + Eventually(func(g Gomega) { + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(broker), broker)).To(Succeed()) + g.Expect(broker.Status.Conditions).To(ContainElement(SatisfyAll( + HasType(Equal(korifiv1alpha1.StatusConditionReady)), + HasStatus(Equal(metav1.ConditionFalse)), + HasReason(Equal("SecretInvalid")), + ))) + }).Should(Succeed()) + }) + }) + + When("the credentials secret data does not have username", func() { + BeforeEach(func() { + Expect(k8s.PatchResource(ctx, adminClient, credentialsSecret, func() { + credentialsSecret.Data = map[string][]byte{ + korifiv1alpha1.CredentialsSecretKey: []byte(`{ "password": "broker-password"}`), + } + })).To(Succeed()) + }) + + It("sets the ready condition to false", func() { + Eventually(func(g Gomega) { + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(broker), broker)).To(Succeed()) + g.Expect(broker.Status.Conditions).To(ContainElement(SatisfyAll( + HasType(Equal(korifiv1alpha1.StatusConditionReady)), + HasStatus(Equal(metav1.ConditionFalse)), + HasReason(Equal("SecretInvalid")), + ))) + }).Should(Succeed()) + }) + }) + + When("the credentials secret data does not have password", func() { + BeforeEach(func() { + Expect(k8s.PatchResource(ctx, adminClient, credentialsSecret, func() { + credentialsSecret.Data = map[string][]byte{ + korifiv1alpha1.CredentialsSecretKey: []byte(`{ "username": "broker-username"}`), + } + })).To(Succeed()) + }) + + It("sets the ready condition to false", func() { + Eventually(func(g Gomega) { + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(broker), broker)).To(Succeed()) + g.Expect(broker.Status.Conditions).To(ContainElement(SatisfyAll( + HasType(Equal(korifiv1alpha1.StatusConditionReady)), + HasStatus(Equal(metav1.ConditionFalse)), + HasReason(Equal("SecretInvalid")), + ))) + }).Should(Succeed()) + }) + }) + + When("the credentials secret is reconciled", func() { + var credentialsObservedVersion string + + BeforeEach(func() { + Eventually(func(g Gomega) { + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(broker), broker)).To(Succeed()) + g.Expect(meta.IsStatusConditionTrue(broker.Status.Conditions, korifiv1alpha1.StatusConditionReady)).To(BeTrue()) + }).Should(Succeed()) + credentialsObservedVersion = broker.Status.CredentialsObservedVersion + }) + + When("the credentials secret changes", func() { + BeforeEach(func() { + Expect(k8s.Patch(ctx, adminClient, credentialsSecret, func() { + credentialsSecret.StringData = map[string]string{"f": "b"} + })).To(Succeed()) + }) + + It("updates the credentials secret observed version", func() { + Eventually(func(g Gomega) { + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(broker), broker)).To(Succeed()) + g.Expect(broker.Status.CredentialsObservedVersion).NotTo(Equal(credentialsObservedVersion)) + }).Should(Succeed()) + }) + }) + + When("the credentials secret gets deleted", func() { + BeforeEach(func() { + Expect(adminClient.Delete(ctx, credentialsSecret)).To(Succeed()) + }) + + It("does not change the credentials secret observed version", func() { + Consistently(func(g Gomega) { + g.Expect(adminClient.Get(ctx, client.ObjectKeyFromObject(broker), broker)).To(Succeed()) + g.Expect(broker.Status.CredentialsObservedVersion).To(Equal(credentialsObservedVersion)) + }).Should(Succeed()) + }) + }) + }) + }) +}) diff --git a/controllers/controllers/services/brokers/suite_test.go b/controllers/controllers/services/brokers/suite_test.go new file mode 100644 index 000000000..e9b871dea --- /dev/null +++ b/controllers/controllers/services/brokers/suite_test.go @@ -0,0 +1,92 @@ +/* +Copyright 2021. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package brokers_test + +import ( + "context" + "path/filepath" + "testing" + "time" + + korifiv1alpha1 "code.cloudfoundry.org/korifi/controllers/api/v1alpha1" + "code.cloudfoundry.org/korifi/controllers/controllers/services/brokers" + "code.cloudfoundry.org/korifi/controllers/controllers/shared" + "code.cloudfoundry.org/korifi/tests/helpers" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "k8s.io/client-go/kubernetes/scheme" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/envtest" + logf "sigs.k8s.io/controller-runtime/pkg/log" + "sigs.k8s.io/controller-runtime/pkg/log/zap" +) + +var ( + ctx context.Context + stopManager context.CancelFunc + stopClientCache context.CancelFunc + testEnv *envtest.Environment + adminClient client.Client +) + +func TestAPIs(t *testing.T) { + SetDefaultEventuallyTimeout(30 * time.Second) + SetDefaultEventuallyPollingInterval(250 * time.Millisecond) + + RegisterFailHandler(Fail) + RunSpecs(t, "Services Broker Controller Integration Suite") +} + +var _ = BeforeSuite(func() { + logf.SetLogger(zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true))) + + ctx, stopManager = context.WithCancel(context.TODO()) + + testEnv = &envtest.Environment{ + CRDDirectoryPaths: []string{ + filepath.Join("..", "..", "..", "..", "helm", "korifi", "controllers", "crds"), + }, + ErrorIfCRDPathMissing: true, + } + + _, err := testEnv.Start() + Expect(err).NotTo(HaveOccurred()) + + Expect(korifiv1alpha1.AddToScheme(scheme.Scheme)).To(Succeed()) + + k8sManager := helpers.NewK8sManager(testEnv, filepath.Join("helm", "korifi", "controllers", "role.yaml")) + Expect(shared.SetupIndexWithManager(k8sManager)).To(Succeed()) + + adminClient, stopClientCache = helpers.NewCachedClient(testEnv.Config) + + err = (brokers.NewReconciler( + k8sManager.GetClient(), + k8sManager.GetScheme(), + ctrl.Log.WithName("controllers").WithName("CFServiceBroker"), + )).SetupWithManager(k8sManager) + Expect(err).ToNot(HaveOccurred()) + + stopManager = helpers.StartK8sManager(k8sManager) +}) + +var _ = AfterSuite(func() { + stopClientCache() + stopManager() + Expect(testEnv.Stop()).To(Succeed()) +}) diff --git a/controllers/controllers/services/credentials/credentials.go b/controllers/controllers/services/credentials/credentials.go index 6cfa07080..94903d792 100644 --- a/controllers/controllers/services/credentials/credentials.go +++ b/controllers/controllers/services/credentials/credentials.go @@ -2,6 +2,7 @@ package credentials import ( "encoding/json" + "errors" "fmt" korifiv1alpha1 "code.cloudfoundry.org/korifi/controllers/api/v1alpha1" @@ -70,3 +71,15 @@ func GetCredentials(credentialsSecret *corev1.Secret) (map[string]any, error) { return credentialsObject, nil } + +func ToCredentialsSecretData(credentials any) (map[string][]byte, error) { + var credentialBytes []byte + credentialBytes, err := json.Marshal(credentials) + if err != nil { + return nil, errors.New("failed to marshal credentials for service instance") + } + + return map[string][]byte{ + korifiv1alpha1.CredentialsSecretKey: credentialBytes, + }, nil +} diff --git a/controllers/controllers/services/credentials/credentials_test.go b/controllers/controllers/services/credentials/credentials_test.go index e8b5dbf2f..846c553f9 100644 --- a/controllers/controllers/services/credentials/credentials_test.go +++ b/controllers/controllers/services/credentials/credentials_test.go @@ -130,4 +130,27 @@ var _ = Describe("Credentials", func() { }) }) }) + + Describe("ToCredentialsSecretData", func() { + var creds any + + BeforeEach(func() { + creds = struct { + Str string + Num int + }{ + Str: "abc", + Num: 5, + } + }) + + It("converts credentials map to a k8s secret data", func() { + secretData, err := credentials.ToCredentialsSecretData(creds) + Expect(err).NotTo(HaveOccurred()) + Expect(secretData).To(SatisfyAll( + HaveLen(1), + HaveKeyWithValue(korifiv1alpha1.CredentialsSecretKey, MatchJSON(`{"Str":"abc", "Num":5}`)), + )) + }) + }) }) diff --git a/controllers/controllers/shared/index.go b/controllers/controllers/shared/index.go index 94f281263..6890007fa 100644 --- a/controllers/controllers/shared/index.go +++ b/controllers/controllers/shared/index.go @@ -22,6 +22,7 @@ const ( IndexAppTasks = "appTasks" IndexSpaceNamespaceName = "spaceNamespace" IndexOrgNamespaceName = "orgNamespace" + IndexServiceBrokerCredentialsSecretName = "serviceBrokerCredentialsSecretName" ) func SetupIndexWithManager(mgr manager.Manager) error { @@ -77,6 +78,14 @@ func SetupIndexWithManager(mgr manager.Manager) error { return err } + err = mgr.GetFieldIndexer().IndexField(context.Background(), &korifiv1alpha1.CFServiceBroker{}, IndexServiceBrokerCredentialsSecretName, func(object client.Object) []string { + serviceBroker := object.(*korifiv1alpha1.CFServiceBroker) + return []string{serviceBroker.Spec.Credentials.Name} + }) + if err != nil { + return err + } + return nil } diff --git a/controllers/main.go b/controllers/main.go index 99ed3f47d..aa43928e3 100644 --- a/controllers/main.go +++ b/controllers/main.go @@ -30,6 +30,7 @@ import ( "code.cloudfoundry.org/korifi/controllers/controllers/networking/domains" "code.cloudfoundry.org/korifi/controllers/controllers/networking/routes" "code.cloudfoundry.org/korifi/controllers/controllers/services/bindings" + "code.cloudfoundry.org/korifi/controllers/controllers/services/brokers" "code.cloudfoundry.org/korifi/controllers/controllers/services/instances" "code.cloudfoundry.org/korifi/controllers/controllers/shared" "code.cloudfoundry.org/korifi/controllers/controllers/workloads/apps" @@ -47,6 +48,7 @@ import ( domainswebhook "code.cloudfoundry.org/korifi/controllers/webhooks/networking/domains" routeswebhook "code.cloudfoundry.org/korifi/controllers/webhooks/networking/routes" bindingswebhook "code.cloudfoundry.org/korifi/controllers/webhooks/services/bindings" + brokerswebhook "code.cloudfoundry.org/korifi/controllers/webhooks/services/brokers" instanceswebhook "code.cloudfoundry.org/korifi/controllers/webhooks/services/instances" "code.cloudfoundry.org/korifi/controllers/webhooks/validation" versionwebhook "code.cloudfoundry.org/korifi/controllers/webhooks/version" @@ -298,6 +300,18 @@ func main() { setupLog.Error(err, "unable to create controller", "controller", "CFDomain") os.Exit(1) } + + if controllerConfig.ExperimentalManagedServicesEnabled { + if err = brokers.NewReconciler( + mgr.GetClient(), + mgr.GetScheme(), + ctrl.Log.WithName("controllers").WithName("CFServiceBroker"), + ).SetupWithManager(mgr); err != nil { + setupLog.Error(err, "unable to create controller", "controller", "CFServiceBroker") + os.Exit(1) + } + } + //+kubebuilder:scaffold:builder // Setup Index with Manager @@ -500,6 +514,13 @@ func main() { os.Exit(1) } + if err = brokerswebhook.NewValidator( + validation.NewDuplicateValidator(coordination.NewNameRegistry(uncachedClient, brokerswebhook.ServiceBrokerEntityType)), + ).SetupWebhookWithManager(mgr); err != nil { + setupLog.Error(err, "unable to create webhook", "webhook", "CFServiceBroker") + os.Exit(1) + } + if err = (&korifiv1alpha1.CFRoute{}).SetupWebhookWithManager(mgr); err != nil { setupLog.Error(err, "unable to create webhook", "webhook", "CFRoute") os.Exit(1) diff --git a/controllers/webhooks/services/brokers/suite_test.go b/controllers/webhooks/services/brokers/suite_test.go new file mode 100644 index 000000000..8dbc5b348 --- /dev/null +++ b/controllers/webhooks/services/brokers/suite_test.go @@ -0,0 +1,23 @@ +package brokers_test + +import ( + "testing" + "time" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + logf "sigs.k8s.io/controller-runtime/pkg/log" + "sigs.k8s.io/controller-runtime/pkg/log/zap" +) + +func TestServicesValidatingWebhooks(t *testing.T) { + SetDefaultEventuallyTimeout(10 * time.Second) + SetDefaultEventuallyPollingInterval(250 * time.Millisecond) + + RegisterFailHandler(Fail) + RunSpecs(t, "CFServiceBroker Webhooks Unit Test Suite") +} + +var _ = BeforeSuite(func() { + logf.SetLogger(zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true))) +}) diff --git a/controllers/webhooks/services/brokers/validator.go b/controllers/webhooks/services/brokers/validator.go new file mode 100644 index 000000000..c14ced83a --- /dev/null +++ b/controllers/webhooks/services/brokers/validator.go @@ -0,0 +1,79 @@ +package brokers + +import ( + "context" + "fmt" + + korifiv1alpha1 "code.cloudfoundry.org/korifi/controllers/api/v1alpha1" + "code.cloudfoundry.org/korifi/controllers/webhooks" + + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/runtime" + ctrl "sigs.k8s.io/controller-runtime" + logf "sigs.k8s.io/controller-runtime/pkg/log" + "sigs.k8s.io/controller-runtime/pkg/webhook" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" +) + +const ( + ServiceBrokerEntityType = "servicebroker" +) + +var cfservicebrokerlog = logf.Log.WithName("cfservicebroker-validator") + +//+kubebuilder:webhook:path=/validate-korifi-cloudfoundry-org-v1alpha1-cfservicebroker,mutating=false,failurePolicy=fail,sideEffects=NoneOnDryRun,groups=korifi.cloudfoundry.org,resources=cfservicebrokers,verbs=create;update;delete,versions=v1alpha1,name=vcfservicebroker.korifi.cloudfoundry.org,admissionReviewVersions={v1,v1beta1} + +func (v *Validator) SetupWebhookWithManager(mgr ctrl.Manager) error { + return ctrl.NewWebhookManagedBy(mgr). + For(&korifiv1alpha1.CFServiceBroker{}). + WithValidator(v). + Complete() +} + +type Validator struct { + duplicateValidator webhooks.NameValidator +} + +var _ webhook.CustomValidator = &Validator{} + +func NewValidator(duplicateValidator webhooks.NameValidator) *Validator { + return &Validator{ + duplicateValidator: duplicateValidator, + } +} + +func (v *Validator) ValidateCreate(ctx context.Context, obj runtime.Object) (admission.Warnings, error) { + serviceBroker, ok := obj.(*korifiv1alpha1.CFServiceBroker) + if !ok { + return nil, apierrors.NewBadRequest(fmt.Sprintf("expected a CFServiceBroker but got a %T", obj)) + } + + return nil, v.duplicateValidator.ValidateCreate(ctx, cfservicebrokerlog, serviceBroker.Namespace, serviceBroker) +} + +func (v *Validator) ValidateUpdate(ctx context.Context, oldObj, obj runtime.Object) (admission.Warnings, error) { + serviceBroker, ok := obj.(*korifiv1alpha1.CFServiceBroker) + if !ok { + return nil, apierrors.NewBadRequest(fmt.Sprintf("expected a CFServiceBroker but got a %T", obj)) + } + + if !serviceBroker.GetDeletionTimestamp().IsZero() { + return nil, nil + } + + oldServiceBroker, ok := oldObj.(*korifiv1alpha1.CFServiceBroker) + if !ok { + return nil, apierrors.NewBadRequest(fmt.Sprintf("expected a CFServiceBroker but got a %T", oldObj)) + } + + return nil, v.duplicateValidator.ValidateUpdate(ctx, cfservicebrokerlog, serviceBroker.Namespace, oldServiceBroker, serviceBroker) +} + +func (v *Validator) ValidateDelete(ctx context.Context, obj runtime.Object) (admission.Warnings, error) { + serviceBroker, ok := obj.(*korifiv1alpha1.CFServiceBroker) + if !ok { + return nil, apierrors.NewBadRequest(fmt.Sprintf("expected a CFServiceBroker but got a %T", obj)) + } + + return nil, v.duplicateValidator.ValidateDelete(ctx, cfservicebrokerlog, serviceBroker.Namespace, serviceBroker) +} diff --git a/controllers/webhooks/services/brokers/validator_test.go b/controllers/webhooks/services/brokers/validator_test.go new file mode 100644 index 000000000..b93e672d2 --- /dev/null +++ b/controllers/webhooks/services/brokers/validator_test.go @@ -0,0 +1,158 @@ +package brokers_test + +import ( + "context" + "errors" + "time" + + korifiv1alpha1 "code.cloudfoundry.org/korifi/controllers/api/v1alpha1" + "code.cloudfoundry.org/korifi/controllers/webhooks/fake" + "code.cloudfoundry.org/korifi/controllers/webhooks/services/brokers" + "code.cloudfoundry.org/korifi/model/services" + + "github.com/google/uuid" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" +) + +var _ = Describe("CFServiceBrokerValidatingWebhook", func() { + const ( + defaultNamespace = "default" + ) + + var ( + ctx context.Context + duplicateValidator *fake.NameValidator + serviceBroker *korifiv1alpha1.CFServiceBroker + validatingWebhook *brokers.Validator + retErr error + ) + + BeforeEach(func() { + ctx = context.Background() + + scheme := runtime.NewScheme() + err := korifiv1alpha1.AddToScheme(scheme) + Expect(err).NotTo(HaveOccurred()) + + serviceBroker = &korifiv1alpha1.CFServiceBroker{ + ObjectMeta: metav1.ObjectMeta{ + Name: uuid.NewString(), + Namespace: defaultNamespace, + }, + Spec: korifiv1alpha1.CFServiceBrokerSpec{ + ServiceBroker: services.ServiceBroker{ + Name: uuid.NewString(), + }, + }, + } + + duplicateValidator = new(fake.NameValidator) + validatingWebhook = brokers.NewValidator(duplicateValidator) + }) + + Describe("ValidateCreate", func() { + JustBeforeEach(func() { + _, retErr = validatingWebhook.ValidateCreate(ctx, serviceBroker) + }) + + It("allows the request", func() { + Expect(retErr).NotTo(HaveOccurred()) + }) + + It("invokes the validator correctly", func() { + Expect(duplicateValidator.ValidateCreateCallCount()).To(Equal(1)) + actualContext, _, actualNamespace, actualResource := duplicateValidator.ValidateCreateArgsForCall(0) + Expect(actualContext).To(Equal(ctx)) + Expect(actualNamespace).To(Equal(serviceBroker.Namespace)) + Expect(actualResource).To(Equal(serviceBroker)) + Expect(actualResource.UniqueValidationErrorMessage()).To(Equal("Name must be unique")) + }) + + When("the broker name is a duplicate", func() { + BeforeEach(func() { + duplicateValidator.ValidateCreateReturns(errors.New("foo")) + }) + + It("denies the request", func() { + Expect(retErr).To(MatchError("foo")) + }) + }) + }) + + Describe("ValidateUpdate", func() { + var updatedServiceBroker *korifiv1alpha1.CFServiceBroker + + BeforeEach(func() { + updatedServiceBroker = serviceBroker.DeepCopy() + updatedServiceBroker.Spec.Name = "the-new-name" + }) + + JustBeforeEach(func() { + _, retErr = validatingWebhook.ValidateUpdate(ctx, serviceBroker, updatedServiceBroker) + }) + + It("allows the request", func() { + Expect(retErr).NotTo(HaveOccurred()) + }) + + It("invokes the validator correctly", func() { + Expect(duplicateValidator.ValidateUpdateCallCount()).To(Equal(1)) + actualContext, _, actualNamespace, oldResource, newResource := duplicateValidator.ValidateUpdateArgsForCall(0) + Expect(actualContext).To(Equal(ctx)) + Expect(actualNamespace).To(Equal(serviceBroker.Namespace)) + Expect(oldResource).To(Equal(serviceBroker)) + Expect(newResource).To(Equal(updatedServiceBroker)) + }) + + When("the service broker is being deleted", func() { + BeforeEach(func() { + updatedServiceBroker.DeletionTimestamp = &metav1.Time{Time: time.Now()} + }) + + It("does not return an error", func() { + Expect(retErr).NotTo(HaveOccurred()) + }) + }) + + When("the new service broker name is a duplicate", func() { + BeforeEach(func() { + duplicateValidator.ValidateUpdateReturns(errors.New("foo")) + }) + + It("denies the request", func() { + Expect(retErr).To(MatchError("foo")) + }) + }) + }) + + Describe("ValidateDelete", func() { + JustBeforeEach(func() { + _, retErr = validatingWebhook.ValidateDelete(ctx, serviceBroker) + }) + + It("allows the request", func() { + Expect(retErr).NotTo(HaveOccurred()) + }) + + It("invokes the validator correctly", func() { + Expect(duplicateValidator.ValidateDeleteCallCount()).To(Equal(1)) + actualContext, _, actualNamespace, actualResource := duplicateValidator.ValidateDeleteArgsForCall(0) + Expect(actualContext).To(Equal(ctx)) + Expect(actualNamespace).To(Equal(serviceBroker.Namespace)) + Expect(actualResource).To(Equal(serviceBroker)) + }) + + When("delete validation fails", func() { + BeforeEach(func() { + duplicateValidator.ValidateDeleteReturns(errors.New("foo")) + }) + + It("disallows the request", func() { + Expect(retErr).To(MatchError("foo")) + }) + }) + }) +}) diff --git a/helm/korifi/controllers/cf_roles/cf_admin.yaml b/helm/korifi/controllers/cf_roles/cf_admin.yaml index bbc035602..009ef934b 100644 --- a/helm/korifi/controllers/cf_roles/cf_admin.yaml +++ b/helm/korifi/controllers/cf_roles/cf_admin.yaml @@ -186,6 +186,14 @@ rules: - list - watch +- apiGroups: + - korifi.cloudfoundry.org + resources: + - cfservicebrokers + verbs: + - create + - get + - apiGroups: - rbac.authorization.k8s.io resources: diff --git a/helm/korifi/controllers/crds/korifi.cloudfoundry.org_cfservicebrokers.yaml b/helm/korifi/controllers/crds/korifi.cloudfoundry.org_cfservicebrokers.yaml new file mode 100644 index 000000000..13864fe86 --- /dev/null +++ b/helm/korifi/controllers/crds/korifi.cloudfoundry.org_cfservicebrokers.yaml @@ -0,0 +1,160 @@ +--- +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + annotations: + controller-gen.kubebuilder.io/version: v0.15.0 + name: cfservicebrokers.korifi.cloudfoundry.org +spec: + group: korifi.cloudfoundry.org + names: + kind: CFServiceBroker + listKind: CFServiceBrokerList + plural: cfservicebrokers + singular: cfservicebroker + scope: Namespaced + versions: + - additionalPrinterColumns: + - jsonPath: .spec.name + name: Broker Name + type: string + - jsonPath: .metadata.creationTimestamp + name: Age + type: date + name: v1alpha1 + schema: + openAPIV3Schema: + properties: + apiVersion: + description: |- + APIVersion defines the versioned schema of this representation of an object. + Servers should convert recognized schemas to the latest internal value, and + may reject unrecognized values. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources + type: string + kind: + description: |- + Kind is a string value representing the REST resource this object represents. + Servers may infer this from the endpoint the client submits requests to. + Cannot be updated. + In CamelCase. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds + type: string + metadata: + type: object + spec: + properties: + credentials: + description: |- + LocalObjectReference contains enough information to let you locate the + referenced object inside the same namespace. + properties: + name: + default: "" + description: |- + Name of the referent. + This field is effectively required, but due to backwards compatibility is + allowed to be empty. Instances of this type with an empty value here are + almost certainly wrong. + TODO: Add other useful fields. apiVersion, kind, uid? + More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + TODO: Drop `kubebuilder:default` when controller-gen doesn't need it https://github.com/kubernetes-sigs/kubebuilder/issues/3896. + type: string + type: object + x-kubernetes-map-type: atomic + name: + type: string + url: + type: string + required: + - credentials + - name + - url + type: object + status: + properties: + conditions: + items: + description: "Condition contains details for one aspect of the current + state of this API Resource.\n---\nThis struct is intended for + direct use as an array at the field path .status.conditions. For + example,\n\n\n\ttype FooStatus struct{\n\t // Represents the + observations of a foo's current state.\n\t // Known .status.conditions.type + are: \"Available\", \"Progressing\", and \"Degraded\"\n\t // + +patchMergeKey=type\n\t // +patchStrategy=merge\n\t // +listType=map\n\t + \ // +listMapKey=type\n\t Conditions []metav1.Condition `json:\"conditions,omitempty\" + patchStrategy:\"merge\" patchMergeKey:\"type\" protobuf:\"bytes,1,rep,name=conditions\"`\n\n\n\t + \ // other fields\n\t}" + properties: + lastTransitionTime: + description: |- + lastTransitionTime is the last time the condition transitioned from one status to another. + This should be when the underlying condition changed. If that is not known, then using the time when the API field changed is acceptable. + format: date-time + type: string + message: + description: |- + message is a human readable message indicating details about the transition. + This may be an empty string. + maxLength: 32768 + type: string + observedGeneration: + description: |- + observedGeneration represents the .metadata.generation that the condition was set based upon. + For instance, if .metadata.generation is currently 12, but the .status.conditions[x].observedGeneration is 9, the condition is out of date + with respect to the current state of the instance. + format: int64 + minimum: 0 + type: integer + reason: + description: |- + reason contains a programmatic identifier indicating the reason for the condition's last transition. + Producers of specific condition types may define expected values and meanings for this field, + and whether the values are considered a guaranteed API. + The value should be a CamelCase string. + This field may not be empty. + maxLength: 1024 + minLength: 1 + pattern: ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$ + type: string + status: + description: status of the condition, one of True, False, Unknown. + enum: + - "True" + - "False" + - Unknown + type: string + type: + description: |- + type of condition in CamelCase or in foo.example.com/CamelCase. + --- + Many .condition.type values are consistent across resources like Available, but because arbitrary conditions can be + useful (see .node.status.conditions), the ability to deconflict is important. + The regex it matches is (dns1123SubdomainFmt/)?(qualifiedNameFmt) + maxLength: 316 + pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$ + type: string + required: + - lastTransitionTime + - message + - reason + - status + - type + type: object + type: array + credentialsObservedVersion: + description: |- + ObservedGeneration captures the latest version of the spec.Credentials.Name secret that has been reconciled + This will ensure that interested contollers are notified on broker credentials change + type: string + observedGeneration: + description: ObservedGeneration captures the latest generation of + the CFServiceBroker that has been reconciled + format: int64 + type: integer + type: object + type: object + served: true + storage: true + subresources: + status: {} diff --git a/helm/korifi/controllers/manifests.yaml b/helm/korifi/controllers/manifests.yaml index a20017ae3..a9b426b0e 100644 --- a/helm/korifi/controllers/manifests.yaml +++ b/helm/korifi/controllers/manifests.yaml @@ -284,6 +284,28 @@ webhooks: resources: - cfservicebindings sideEffects: NoneOnDryRun + - admissionReviewVersions: + - v1 + - v1beta1 + clientConfig: + service: + name: korifi-controllers-webhook-service + namespace: '{{ .Release.Namespace }}' + path: /validate-korifi-cloudfoundry-org-v1alpha1-cfservicebroker + failurePolicy: Fail + name: vcfservicebroker.korifi.cloudfoundry.org + rules: + - apiGroups: + - korifi.cloudfoundry.org + apiVersions: + - v1alpha1 + operations: + - CREATE + - UPDATE + - DELETE + resources: + - cfservicebrokers + sideEffects: NoneOnDryRun - admissionReviewVersions: - v1 - v1beta1 diff --git a/helm/korifi/controllers/role.yaml b/helm/korifi/controllers/role.yaml index 1ebabe33a..c79c46166 100644 --- a/helm/korifi/controllers/role.yaml +++ b/helm/korifi/controllers/role.yaml @@ -393,6 +393,26 @@ rules: - get - patch - update +- apiGroups: + - korifi.cloudfoundry.org + resources: + - cfservicebrokers + verbs: + - create + - delete + - get + - list + - patch + - update + - watch +- apiGroups: + - korifi.cloudfoundry.org + resources: + - cfservicebrokers/status + verbs: + - get + - patch + - update - apiGroups: - korifi.cloudfoundry.org resources: diff --git a/helm/korifi/values.schema.json b/helm/korifi/values.schema.json index bacb1e006..75d451416 100644 --- a/helm/korifi/values.schema.json +++ b/helm/korifi/values.schema.json @@ -599,7 +599,7 @@ "type": "object" } }, - "description": "Experimental features. Make sure you do not enable those on production. No guarantee provided! Backwards incompatible changes in future are quite probable!", + "description": "Experimental features. No guarantees are provided and breaking/backwards incompatible changes should be expected. These features are not recommended for use in production environments.", "type": "object" } }, diff --git a/model/cfresource.go b/model/cfresource.go new file mode 100644 index 000000000..5dae52b03 --- /dev/null +++ b/model/cfresource.go @@ -0,0 +1,22 @@ +package model + +import "time" + +type CFResourceStatus int + +const ( + CFResourceStatusUnknown CFResourceStatus = iota + CFResourceStatusReady +) + +type CFResourceState struct { + Status CFResourceStatus + Details string +} + +type CFResource struct { + GUID string `json:"guid"` + CreatedAt time.Time `json:"created_at"` + UpdatedAt *time.Time `json:"updated_at"` + State CFResourceState `json:"state"` +} diff --git a/model/metadata.go b/model/metadata.go new file mode 100644 index 000000000..c3e7015f4 --- /dev/null +++ b/model/metadata.go @@ -0,0 +1,6 @@ +package model + +type Metadata struct { + Labels map[string]string `json:"labels,omitempty"` + Annotations map[string]string `json:"annotations,omitempty"` +} diff --git a/model/services/brokers.go b/model/services/brokers.go new file mode 100644 index 000000000..c8f55eea9 --- /dev/null +++ b/model/services/brokers.go @@ -0,0 +1,12 @@ +package services + +// +kubebuilder:object:generate=true +type ServiceBroker struct { + Name string `json:"name"` + URL string `json:"url"` +} + +type BrokerCredentials struct { + Username string `json:"username"` + Password string `json:"password"` +} diff --git a/model/services/zz_generated.deepcopy.go b/model/services/zz_generated.deepcopy.go new file mode 100644 index 000000000..38e1d072e --- /dev/null +++ b/model/services/zz_generated.deepcopy.go @@ -0,0 +1,38 @@ +//go:build !ignore_autogenerated + +/* +Copyright 2021. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +// Code generated by controller-gen. DO NOT EDIT. + +package services + +import () + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ServiceBroker) DeepCopyInto(out *ServiceBroker) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ServiceBroker. +func (in *ServiceBroker) DeepCopy() *ServiceBroker { + if in == nil { + return nil + } + out := new(ServiceBroker) + in.DeepCopyInto(out) + return out +} diff --git a/model/zz_generated.deepcopy.go b/model/zz_generated.deepcopy.go new file mode 100644 index 000000000..b60423210 --- /dev/null +++ b/model/zz_generated.deepcopy.go @@ -0,0 +1,52 @@ +//go:build !ignore_autogenerated + +/* +Copyright 2021. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +// Code generated by controller-gen. DO NOT EDIT. + +package model + +import () + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *Metadata) DeepCopyInto(out *Metadata) { + *out = *in + if in.Labels != nil { + in, out := &in.Labels, &out.Labels + *out = make(map[string]string, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } + if in.Annotations != nil { + in, out := &in.Annotations, &out.Annotations + *out = make(map[string]string, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Metadata. +func (in *Metadata) DeepCopy() *Metadata { + if in == nil { + return nil + } + out := new(Metadata) + in.DeepCopyInto(out) + return out +} diff --git a/tests/e2e/e2e_suite_test.go b/tests/e2e/e2e_suite_test.go index 7b08032e4..83472a359 100644 --- a/tests/e2e/e2e_suite_test.go +++ b/tests/e2e/e2e_suite_test.go @@ -227,6 +227,17 @@ type serviceInstanceResource struct { InstanceType string `json:"type"` } +type serviceBrokerResource struct { + resource `json:",inline"` + URL string `json:"url"` + Authentication serviceBrokerAuthenticationResource `json:"authentication"` +} + +type serviceBrokerAuthenticationResource struct { + Type string `json:"type"` + Credentials map[string]any `json:"credentials"` +} + type appLogResource struct { Envelopes appLogResourceEnvelopes `json:"envelopes"` } diff --git a/tests/e2e/service_brokers_test.go b/tests/e2e/service_brokers_test.go new file mode 100644 index 000000000..f503ed4ac --- /dev/null +++ b/tests/e2e/service_brokers_test.go @@ -0,0 +1,90 @@ +package e2e_test + +import ( + "context" + "net/http" + "strings" + + korifiv1alpha1 "code.cloudfoundry.org/korifi/controllers/api/v1alpha1" + "github.com/go-resty/resty/v2" + "github.com/google/uuid" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes/scheme" + "k8s.io/client-go/rest" + controllerruntime "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +var _ = Describe("Service Brokers", func() { + var resp *resty.Response + + Describe("Create", func() { + var err error + + BeforeEach(func() { + Expect(korifiv1alpha1.AddToScheme(scheme.Scheme)).To(Succeed()) + }) + + JustBeforeEach(func() { + resp, err = adminClient.R(). + SetBody(serviceBrokerResource{ + resource: resource{ + Name: uuid.NewString(), + }, + URL: "https://my.broker.com", + Authentication: serviceBrokerAuthenticationResource{ + Type: "basic", + Credentials: map[string]any{ + "username": "broker-user", + "password": "broker-password", + }, + }, + }). + Post("/v3/service_brokers") + Expect(err).NotTo(HaveOccurred()) + }) + + It("succeeds with a job redirect", func() { + Expect(resp).To(SatisfyAll( + HaveRestyStatusCode(http.StatusAccepted), + HaveRestyHeaderWithValue("Location", ContainSubstring("/v3/jobs/service_broker.create~")), + )) + + locationSplit := strings.Split(resp.Header().Get("Location"), "~") + DeferCleanup(func() { + // Temporarily delete the broker created by the test via the k8s client + // Once the API supports broker deletion, we could get rid of this + if len(locationSplit) != 2 { + return + } + + var config *rest.Config + config, err = controllerruntime.GetConfig() + Expect(err).NotTo(HaveOccurred()) + + var k8sClient client.Client + k8sClient, err = client.New(config, client.Options{Scheme: scheme.Scheme}) + Expect(err).NotTo(HaveOccurred()) + + broker := &korifiv1alpha1.CFServiceBroker{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: rootNamespace, + Name: locationSplit[1], + }, + } + + Expect(k8sClient.Delete(context.Background(), broker)).To(Succeed()) + }) + + jobURL := resp.Header().Get("Location") + Eventually(func(g Gomega) { + resp, err = adminClient.R().Get(jobURL) + g.Expect(err).NotTo(HaveOccurred()) + jobRespBody := string(resp.Body()) + g.Expect(jobRespBody).To(ContainSubstring("COMPLETE")) + }).Should(Succeed()) + }) + }) +})