Skip to content

Commit

Permalink
Use valid UUIDs for Korifi resources guids
Browse files Browse the repository at this point in the history
* Do not prefix uuids for orgs, spaces and routes
* Use uuidv5 for CFProcess guids for optimistic locking. The app GUID is
  used as a UUID namespace, the process type is used as name
* Document the decision in an ADR

fixes #2330, #2177

Co-authored-by: Danail Branekov <danailster@gmail.com>
  • Loading branch information
georgethebeatle and danail-branekov committed Nov 21, 2024
1 parent 723ce24 commit cd3252c
Show file tree
Hide file tree
Showing 36 changed files with 157 additions and 95 deletions.
34 changes: 19 additions & 15 deletions api/repositories/app_repository.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,6 @@ const (
StartedState DesiredState = "STARTED"
StoppedState DesiredState = "STOPPED"

Kind string = "CFApp"
APIVersion string = "korifi.cloudfoundry.org/v1alpha1"
CFAppGUIDLabel string = "korifi.cloudfoundry.org/app-guid"
AppResourceType string = "App"
AppEnvResourceType string = "App Env"
Expand Down Expand Up @@ -292,7 +290,7 @@ func (f *AppRepo) CreateApp(ctx context.Context, authInfo authorization.Info, ap

envSecret := &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: GenerateEnvSecretName(cfApp.Name),
Name: cfApp.Spec.EnvSecretName,
Namespace: cfApp.Namespace,
Labels: map[string]string{
CFAppGUIDLabel: cfApp.Name,
Expand Down Expand Up @@ -388,16 +386,27 @@ func (f *AppRepo) ListApps(ctx context.Context, authInfo authorization.Info, mes
}

func (f *AppRepo) PatchAppEnvVars(ctx context.Context, authInfo authorization.Info, message PatchAppEnvVarsMessage) (AppEnvVarsRecord, error) {
secretObj := corev1.Secret{
userClient, err := f.userClientFactory.BuildClient(authInfo)
if err != nil {
return AppEnvVarsRecord{}, fmt.Errorf("failed to build user client: %w", err)
}

cfApp := &korifiv1alpha1.CFApp{
ObjectMeta: metav1.ObjectMeta{
Name: GenerateEnvSecretName(message.AppGUID),
Namespace: message.SpaceGUID,
Name: message.AppGUID,
},
}

userClient, err := f.userClientFactory.BuildClient(authInfo)
err = userClient.Get(ctx, client.ObjectKeyFromObject(cfApp), cfApp)
if err != nil {
return AppEnvVarsRecord{}, fmt.Errorf("failed to build user client: %w", err)
return AppEnvVarsRecord{}, apierrors.FromK8sError(err, AppEnvResourceType)
}

secretObj := corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: cfApp.Spec.EnvSecretName,
Namespace: message.SpaceGUID,
},
}

err = PatchResource(ctx, userClient, &secretObj, func() {
Expand Down Expand Up @@ -617,23 +626,18 @@ func getAppEnv(ctx context.Context, userClient client.Client, app AppRecord) (ma
return appEnvMap, nil
}

func GenerateEnvSecretName(appGUID string) string {
return appGUID + "-env"
}

func (m *CreateAppMessage) toCFApp() korifiv1alpha1.CFApp {
guid := uuid.NewString()
return korifiv1alpha1.CFApp{
ObjectMeta: metav1.ObjectMeta{
Name: guid,
Name: uuid.NewString(),
Namespace: m.SpaceGUID,
Labels: m.Labels,
Annotations: m.Annotations,
},
Spec: korifiv1alpha1.CFAppSpec{
DisplayName: m.Name,
DesiredState: korifiv1alpha1.AppState(m.State),
EnvSecretName: GenerateEnvSecretName(guid),
EnvSecretName: uuid.NewString(),
Lifecycle: korifiv1alpha1.Lifecycle{
Type: korifiv1alpha1.LifecycleType(m.Lifecycle.Type),
Data: korifiv1alpha1.LifecycleData{
Expand Down
22 changes: 11 additions & 11 deletions api/repositories/app_repository_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -742,21 +742,18 @@ var _ = Describe("AppRepository", func() {
)

var (
envSecretName string
secretRecord repositories.AppEnvVarsRecord
patchErr error
secretRecord repositories.AppEnvVarsRecord
patchErr error
)

BeforeEach(func() {
envSecretName = cfApp.Name + "-env"

envVars := map[string]string{
key0: "VAL0",
key1: "original-value",
}
secret := corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: repositories.GenerateEnvSecretName(cfApp.Name),
Name: cfApp.Spec.EnvSecretName,
Namespace: cfSpace.Name,
},
StringData: envVars,
Expand Down Expand Up @@ -796,13 +793,16 @@ var _ = Describe("AppRepository", func() {
})

It("patches the underlying secret", func() {
cfAppSecretLookupKey := types.NamespacedName{Name: envSecretName, Namespace: cfSpace.Name}

var updatedSecret corev1.Secret
err := k8sClient.Get(ctx, cfAppSecretLookupKey, &updatedSecret)
envSecret := corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: cfApp.Spec.EnvSecretName,
Namespace: cfSpace.Name,
},
}
err := k8sClient.Get(ctx, client.ObjectKeyFromObject(&envSecret), &envSecret)
Expect(err).NotTo(HaveOccurred())

Expect(asMapOfStrings(updatedSecret.Data)).To(SatisfyAll(
Expect(asMapOfStrings(envSecret.Data)).To(SatisfyAll(
HaveLen(2),
HaveKeyWithValue(key0, "VAL0"),
HaveKeyWithValue(key2, "VAL2"),
Expand Down
3 changes: 1 addition & 2 deletions api/repositories/build_repository.go
Original file line number Diff line number Diff line change
Expand Up @@ -197,10 +197,9 @@ type CreateBuildMessage struct {
}

func (m CreateBuildMessage) toCFBuild() korifiv1alpha1.CFBuild {
guid := uuid.NewString()
return korifiv1alpha1.CFBuild{
ObjectMeta: metav1.ObjectMeta{
Name: guid,
Name: uuid.NewString(),
Namespace: m.SpaceGUID,
Labels: m.Labels,
Annotations: m.Annotations,
Expand Down
2 changes: 1 addition & 1 deletion api/repositories/build_repository_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -479,7 +479,7 @@ var _ = Describe("BuildRepository", func() {
It("returns correct build record", func() {
Expect(buildCreateErr).NotTo(HaveOccurred())

Expect(buildCreateRecord.GUID).NotTo(BeEmpty())
Expect(buildCreateRecord.GUID).To(matchers.BeValidUUID())
Expect(buildCreateRecord.Lifecycle.Type).To(Equal("docker"))
Expect(buildCreateRecord.Lifecycle.Data).To(Equal(repositories.LifecycleData{}))
})
Expand Down
2 changes: 1 addition & 1 deletion api/repositories/domain_repository_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ var _ = Describe("DomainRepository", func() {
Expect(createErr).NotTo(HaveOccurred())

createdDomainGUID := createdDomain.GUID
Expect(createdDomainGUID).NotTo(BeEmpty())
Expect(createdDomainGUID).To(matchers.BeValidUUID())
Expect(createdDomain.Name).To(Equal("my.domain"))
Expect(createdDomain.Labels).To(HaveKeyWithValue("foo", "bar"))
Expect(createdDomain.Annotations).To(HaveKeyWithValue("bar", "baz"))
Expand Down
3 changes: 1 addition & 2 deletions api/repositories/org_repository.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import (
)

const (
OrgPrefix = "cf-org-"
OrgResourceType = "Org"
)

Expand Down Expand Up @@ -95,7 +94,7 @@ func (r *OrgRepo) CreateOrg(ctx context.Context, info authorization.Info, messag

cfOrg := &korifiv1alpha1.CFOrg{
ObjectMeta: metav1.ObjectMeta{
Name: OrgPrefix + uuid.NewString(),
Name: uuid.NewString(),
Namespace: r.rootNamespace,
Labels: message.Labels,
Annotations: message.Annotations,
Expand Down
2 changes: 1 addition & 1 deletion api/repositories/org_repository_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ var _ = Describe("OrgRepository", func() {
Expect(createErr).NotTo(HaveOccurred())

Expect(orgRecord.Name).To(Equal(orgGUID))
Expect(orgRecord.GUID).To(HavePrefix("cf-org-"))
Expect(orgRecord.GUID).To(matchers.BeValidUUID())
Expect(orgRecord.CreatedAt).To(BeTemporally("~", time.Now(), timeCheckThreshold))
Expect(orgRecord.UpdatedAt).To(PointTo(BeTemporally("~", time.Now(), timeCheckThreshold)))
Expect(orgRecord.DeletedAt).To(BeNil())
Expand Down
3 changes: 1 addition & 2 deletions api/repositories/package_repository.go
Original file line number Diff line number Diff line change
Expand Up @@ -172,14 +172,13 @@ type PackageData struct {
}

func (message CreatePackageMessage) toCFPackage() *korifiv1alpha1.CFPackage {
packageGUID := uuid.NewString()
pkg := &korifiv1alpha1.CFPackage{
TypeMeta: metav1.TypeMeta{
Kind: kind,
APIVersion: korifiv1alpha1.SchemeGroupVersion.Identifier(),
},
ObjectMeta: metav1.ObjectMeta{
Name: packageGUID,
Name: uuid.NewString(),
Namespace: message.SpaceGUID,
Labels: message.Metadata.Labels,
Annotations: message.Metadata.Annotations,
Expand Down
4 changes: 2 additions & 2 deletions api/repositories/package_repository_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ var _ = Describe("PackageRepository", func() {
Expect(createErr).NotTo(HaveOccurred())

packageGUID := createdPackage.GUID
Expect(packageGUID).NotTo(BeEmpty())
Expect(packageGUID).To(matchers.BeValidUUID())
Expect(createdPackage.Type).To(Equal("bits"))
Expect(createdPackage.AppGUID).To(Equal(appGUID))
Expect(createdPackage.State).To(Equal("AWAITING_UPLOAD"))
Expand Down Expand Up @@ -244,7 +244,7 @@ var _ = Describe("PackageRepository", func() {
Expect(createErr).NotTo(HaveOccurred())

packageGUID := createdPackage.GUID
Expect(packageGUID).NotTo(BeEmpty())
Expect(packageGUID).To(matchers.BeValidUUID())
Expect(createdPackage.Type).To(Equal("docker"))
Expect(createdPackage.AppGUID).To(Equal(appGUID))
Expect(createdPackage.Labels).To(HaveKeyWithValue("bob", "foo"))
Expand Down
2 changes: 1 addition & 1 deletion api/repositories/process_repository.go
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,7 @@ func (r *ProcessRepo) CreateProcess(ctx context.Context, authInfo authorization.
process := &korifiv1alpha1.CFProcess{
ObjectMeta: metav1.ObjectMeta{
Namespace: message.SpaceGUID,
Name: tools.NamespacedUUID(message.AppGUID, message.Type),
},
Spec: korifiv1alpha1.CFProcessSpec{
AppRef: corev1.LocalObjectReference{Name: message.AppGUID},
Expand All @@ -231,7 +232,6 @@ func (r *ProcessRepo) CreateProcess(ctx context.Context, authInfo authorization.
DiskQuotaMB: message.DiskQuotaMB,
},
}
process.SetStableName(message.AppGUID)
err = userClient.Create(ctx, process)
return apierrors.FromK8sError(err, ProcessResourceType)
}
Expand Down
17 changes: 15 additions & 2 deletions api/repositories/process_repository_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -363,8 +363,7 @@ var _ = Describe("ProcessRepo", func() {
Expect(list.Items).To(HaveLen(1))

process := list.Items[0]
Expect(process.Name).To(HavePrefix("cf-proc-"))
Expect(process.Name).To(HaveSuffix("-web"))
Expect(process.Name).NotTo(BeEmpty())
Expect(process.Spec).To(Equal(korifiv1alpha1.CFProcessSpec{
AppRef: corev1.LocalObjectReference{Name: app1GUID},
ProcessType: "web",
Expand All @@ -382,6 +381,20 @@ var _ = Describe("ProcessRepo", func() {
DiskQuotaMB: 123,
}))
})

When("a process with that process type already exists", func() {
BeforeEach(func() {
Expect(processRepo.CreateProcess(ctx, authInfo, repositories.CreateProcessMessage{
AppGUID: app1GUID,
SpaceGUID: space.Name,
Type: "web",
})).To(Succeed())
})

It("returns an already exists error", func() {
Expect(createErr).To(MatchError(ContainSubstring("already exists")))
})
})
})

When("the user is not authorized in the space", func() {
Expand Down
2 changes: 1 addition & 1 deletion api/repositories/repositories_suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -422,7 +422,7 @@ func createAppWithGUID(space, guid string) *korifiv1alpha1.CFApp {
CurrentDropletRef: corev1.LocalObjectReference{
Name: uuid.NewString(),
},
EnvSecretName: repositories.GenerateEnvSecretName(guid),
EnvSecretName: uuid.NewString(),
},
}
Expect(k8sClient.Create(context.Background(), cfApp)).To(Succeed())
Expand Down
7 changes: 1 addition & 6 deletions api/repositories/route_repository.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import (

const (
RouteResourceType = "Route"
RoutePrefix = "cf-route-"
)

type RouteRepo struct {
Expand Down Expand Up @@ -149,12 +148,8 @@ type DeleteRouteMessage struct {

func (m CreateRouteMessage) toCFRoute() korifiv1alpha1.CFRoute {
return korifiv1alpha1.CFRoute{
TypeMeta: metav1.TypeMeta{
Kind: Kind,
APIVersion: APIVersion,
},
ObjectMeta: metav1.ObjectMeta{
Name: RoutePrefix + uuid.NewString(),
Name: uuid.NewString(),
Namespace: m.SpaceGUID,
Labels: m.Labels,
Annotations: m.Annotations,
Expand Down
4 changes: 2 additions & 2 deletions api/repositories/route_repository_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -576,7 +576,7 @@ var _ = Describe("RouteRepository", func() {
})

It("returns a RouteRecord with matching fields", func() {
Expect(createdRouteRecord.GUID).To(HavePrefix("cf-route-"))
Expect(createdRouteRecord.GUID).To(matchers.BeValidUUID())
Expect(createdRouteRecord.Host).To(Equal(routeHost), "Route Host in record did not match input")
Expect(createdRouteRecord.Path).To(Equal(routePath), "Route Path in record did not match input")
Expect(createdRouteRecord.SpaceGUID).To(Equal(space.Name), "Route Space GUID in record did not match input")
Expand Down Expand Up @@ -711,7 +711,7 @@ var _ = Describe("RouteRepository", func() {

It("returns an RouteRecord with matching fields", func() {
Expect(routeErr).NotTo(HaveOccurred())
Expect(routeRecord.GUID).To(HavePrefix("cf-route-"))
Expect(routeRecord.GUID).To(matchers.BeValidUUID())
Expect(routeRecord.Host).To(Equal(routeHost), "Route Host in record did not match input")
Expect(routeRecord.Path).To(Equal(routePath), "Route Path in record did not match input")
Expect(routeRecord.SpaceGUID).To(Equal(space.Name), "Route Space GUID in record did not match input")
Expand Down
3 changes: 1 addition & 2 deletions api/repositories/service_binding_repository.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,10 +111,9 @@ func (m *ListServiceBindingsMessage) matches(serviceBinding korifiv1alpha1.CFSer
}

func (m CreateServiceBindingMessage) toCFServiceBinding() *korifiv1alpha1.CFServiceBinding {
guid := uuid.NewString()
return &korifiv1alpha1.CFServiceBinding{
ObjectMeta: metav1.ObjectMeta{
Name: guid,
Name: uuid.NewString(),
Namespace: m.SpaceGUID,
Labels: map[string]string{LabelServiceBindingProvisionedService: "true"},
},
Expand Down
4 changes: 2 additions & 2 deletions api/repositories/service_binding_repository_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,7 @@ var _ = Describe("ServiceBindingRepo", func() {
It("creates a new CFServiceBinding resource and returns a record", func() {
Expect(createErr).NotTo(HaveOccurred())

Expect(serviceBindingRecord.GUID).NotTo(BeEmpty())
Expect(serviceBindingRecord.GUID).To(matchers.BeValidUUID())
Expect(serviceBindingRecord.Type).To(Equal("app"))
Expect(serviceBindingRecord.Name).To(BeNil())
Expect(serviceBindingRecord.AppGUID).To(Equal(appGUID))
Expand Down Expand Up @@ -523,7 +523,7 @@ var _ = Describe("ServiceBindingRepo", func() {
It("creates a new CFServiceBinding resource and returns a record", func() {
Expect(createErr).NotTo(HaveOccurred())

Expect(serviceBindingRecord.GUID).NotTo(BeEmpty())
Expect(serviceBindingRecord.GUID).To(matchers.BeValidUUID())
Expect(serviceBindingRecord.Type).To(Equal("app"))
Expect(serviceBindingRecord.Name).To(BeNil())
Expect(serviceBindingRecord.AppGUID).To(Equal(appGUID))
Expand Down
2 changes: 1 addition & 1 deletion api/repositories/service_broker_repository_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ var _ = Describe("ServiceBrokerRepo", func() {
"annotation": "annotation-value",
},
}))
Expect(brokerRecord.CFResource.GUID).NotTo(BeEmpty())
Expect(brokerRecord.GUID).To(BeValidUUID())
Expect(brokerRecord.CFResource.CreatedAt).NotTo(BeZero())
})

Expand Down
5 changes: 2 additions & 3 deletions api/repositories/service_instance_repository.go
Original file line number Diff line number Diff line change
Expand Up @@ -201,17 +201,16 @@ func (r *ServiceInstanceRepo) CreateUserProvidedServiceInstance(ctx context.Cont
return ServiceInstanceRecord{}, fmt.Errorf("failed to build user client: %w", err)
}

guid := uuid.NewString()
cfServiceInstance := &korifiv1alpha1.CFServiceInstance{
ObjectMeta: metav1.ObjectMeta{
Name: guid,
Name: uuid.NewString(),
Namespace: message.SpaceGUID,
Labels: message.Labels,
Annotations: message.Annotations,
},
Spec: korifiv1alpha1.CFServiceInstanceSpec{
DisplayName: message.Name,
SecretName: guid,
SecretName: uuid.NewString(),
Type: korifiv1alpha1.UserProvidedType,
Tags: message.Tags,
},
Expand Down
4 changes: 2 additions & 2 deletions api/repositories/service_instance_repository_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ var _ = Describe("ServiceInstanceRepository", func() {
It("returns a service instance record", func() {
Expect(createErr).NotTo(HaveOccurred())

Expect(record.GUID).NotTo(BeEmpty())
Expect(record.GUID).To(matchers.BeValidUUID())
Expect(record.SpaceGUID).To(Equal(space.Name))
Expect(record.Name).To(Equal(serviceInstanceName))
Expect(record.Type).To(Equal("user-provided"))
Expand Down Expand Up @@ -203,7 +203,7 @@ var _ = Describe("ServiceInstanceRepository", func() {
It("returns a service instance record", func() {
Expect(createErr).NotTo(HaveOccurred())

Expect(record.GUID).NotTo(BeEmpty())
Expect(record.GUID).To(matchers.BeValidUUID())
Expect(record.SpaceGUID).To(Equal(space.Name))
Expect(record.Name).To(Equal(serviceInstanceName))
Expect(record.Type).To(Equal("managed"))
Expand Down
Loading

0 comments on commit cd3252c

Please sign in to comment.