Skip to content

Commit 7a028ce

Browse files
migmartrimigmartri
authored
feat(controlplane): make organization names unique (#525)
Signed-off-by: Miguel Martinez Trivino <miguel@chainloop.dev>
1 parent 438621b commit 7a028ce

21 files changed

Lines changed: 173 additions & 73 deletions

‎app/controlplane/internal/biz/apitoken_integration_test.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,7 @@ func (s *apiTokenTestSuite) TestList() {
161161
})
162162

163163
s.T().Run("returns empty list", func(t *testing.T) {
164-
emptyOrg, err := s.Organization.Create(ctx, "org1")
164+
emptyOrg, err := s.Organization.CreateWithRandomName(ctx)
165165
require.NoError(s.T(), err)
166166
tokens, err := s.APIToken.List(ctx, emptyOrg.ID, false)
167167
s.NoError(err)
@@ -240,9 +240,9 @@ func (s *apiTokenTestSuite) SetupTest() {
240240
ctx := context.Background()
241241

242242
s.TestingUseCases = testhelpers.NewTestingUseCases(t)
243-
s.org, err = s.Organization.Create(ctx, "org1")
243+
s.org, err = s.Organization.CreateWithRandomName(ctx)
244244
assert.NoError(err)
245-
s.org2, err = s.Organization.Create(ctx, "org2")
245+
s.org2, err = s.Organization.CreateWithRandomName(ctx)
246246
assert.NoError(err)
247247

248248
// Create 2 tokens for org 1

‎app/controlplane/internal/biz/biz.go‎

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,15 @@
1515

1616
package biz
1717

18-
import "github.com/google/wire"
18+
import (
19+
"crypto/rand"
20+
"fmt"
21+
"math/big"
22+
"strings"
23+
24+
"github.com/google/wire"
25+
"github.com/moby/moby/pkg/namesgenerator"
26+
)
1927

2028
// ProviderSet is biz providers.
2129
var ProviderSet = wire.NewSet(
@@ -42,3 +50,19 @@ var ProviderSet = wire.NewSet(
4250
wire.Struct(new(NewIntegrationUseCaseOpts), "*"),
4351
wire.Struct(new(NewUserUseCaseParams), "*"),
4452
)
53+
54+
// generate a DNS1123-valid random name using moby's namesgenerator
55+
// plus an additional random number
56+
func generateRandomName() (string, error) {
57+
// Create a random name
58+
name := namesgenerator.GetRandomName(0)
59+
// and append a random number to it
60+
randomNumber, err := rand.Int(rand.Reader, big.NewInt(10000))
61+
if err != nil {
62+
return "", fmt.Errorf("failed to generate random number: %w", err)
63+
}
64+
65+
// Replace underscores with dashes to make it compatible with DNS1123
66+
name = strings.ReplaceAll(fmt.Sprintf("%s-%d", name, randomNumber), "_", "-")
67+
return name, nil
68+
}

‎app/controlplane/internal/biz/errors.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ import (
2020
"fmt"
2121
)
2222

23+
var ErrAlreadyExists = errors.New("duplicate")
24+
2325
type ErrNotFound struct {
2426
entity string
2527
}

‎app/controlplane/internal/biz/membership_integration_test.go‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -34,9 +34,9 @@ func (s *membershipIntegrationTestSuite) TestDeleteWithOrg() {
3434
s.NoError(err)
3535
user2, err := s.User.FindOrCreateByEmail(ctx, "foo-2@test.com")
3636
s.NoError(err)
37-
userOrg, err := s.Organization.Create(ctx, "foo")
37+
userOrg, err := s.Organization.CreateWithRandomName(ctx)
3838
s.NoError(err)
39-
sharedOrg, err := s.Organization.Create(ctx, "shared-org")
39+
sharedOrg, err := s.Organization.CreateWithRandomName(ctx)
4040
s.NoError(err)
4141

4242
mUser, err := s.Membership.Create(ctx, userOrg.ID, user.ID, true)
@@ -100,7 +100,7 @@ func (s *membershipIntegrationTestSuite) TestCreateMembership() {
100100
assert.NoError(err)
101101

102102
s.T().Run("Create default", func(t *testing.T) {
103-
org, err := s.Organization.Create(ctx, "foo")
103+
org, err := s.Organization.CreateWithRandomName(ctx)
104104
assert.NoError(err)
105105

106106
m, err := s.Membership.Create(ctx, org.ID, user.ID, true)
@@ -119,7 +119,7 @@ func (s *membershipIntegrationTestSuite) TestCreateMembership() {
119119
})
120120

121121
s.T().Run("Non current", func(t *testing.T) {
122-
org, err := s.Organization.Create(ctx, "foo")
122+
org, err := s.Organization.CreateWithRandomName(ctx)
123123
assert.NoError(err)
124124

125125
m, err := s.Membership.Create(ctx, org.ID, user.ID, false)
@@ -134,7 +134,7 @@ func (s *membershipIntegrationTestSuite) TestCreateMembership() {
134134
})
135135

136136
s.T().Run("Invalid User", func(t *testing.T) {
137-
org, err := s.Organization.Create(ctx, "foo")
137+
org, err := s.Organization.CreateWithRandomName(ctx)
138138
assert.NoError(err)
139139
m, err := s.Membership.Create(ctx, org.ID, uuid.NewString(), false)
140140
assert.Error(err)

‎app/controlplane/internal/biz/organization.go‎

Lines changed: 51 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,10 @@ import (
1919
"context"
2020
"errors"
2121
"fmt"
22-
"strings"
2322
"time"
2423

2524
"github.com/go-kratos/kratos/v2/log"
2625
"github.com/google/uuid"
27-
"github.com/moby/moby/pkg/namesgenerator"
2826
"k8s.io/apimachinery/pkg/util/validation"
2927
)
3028

@@ -57,22 +55,64 @@ func NewOrganizationUseCase(repo OrganizationRepo, repoUC *CASBackendUseCase, iU
5755
}
5856
}
5957

58+
const OrganizationRandomNameMaxTries = 10
59+
60+
func (uc *OrganizationUseCase) CreateWithRandomName(ctx context.Context) (*Organization, error) {
61+
// Try 10 times to create a random name
62+
for i := 0; i < OrganizationRandomNameMaxTries; i++ {
63+
// Create a random name
64+
name, err := generateRandomName()
65+
if err != nil {
66+
return nil, fmt.Errorf("failed to generate random name: %w", err)
67+
}
68+
69+
org, err := uc.doCreate(ctx, name)
70+
if err != nil {
71+
// We retry if the organization already exists
72+
if errors.Is(err, ErrAlreadyExists) {
73+
uc.logger.Debugw("msg", "Org exists!", "name", name)
74+
continue
75+
}
76+
uc.logger.Debugw("msg", "BOOM", "name", name, "err", err)
77+
return nil, err
78+
}
79+
80+
return org, nil
81+
}
82+
83+
return nil, errors.New("failed to create a random organization name")
84+
}
85+
86+
// Create an organization with the given name
6087
func (uc *OrganizationUseCase) Create(ctx context.Context, name string) (*Organization, error) {
61-
// Create a random name if none is provided
62-
if name == "" {
63-
name = namesgenerator.GetRandomName(0)
64-
// Replace underscores with dashes to make it compatible with DNS1123
65-
name = strings.ReplaceAll(name, "_", "-")
88+
org, err := uc.doCreate(ctx, name)
89+
if err != nil {
90+
if errors.Is(err, ErrAlreadyExists) {
91+
return nil, NewErrValidationStr("organization already exists")
92+
}
93+
94+
return nil, fmt.Errorf("failed to create organization: %w", err)
6695
}
6796

68-
if err := validateOrgName(name); err != nil {
97+
return org, nil
98+
}
99+
100+
func (uc *OrganizationUseCase) doCreate(ctx context.Context, name string) (*Organization, error) {
101+
uc.logger.Infow("msg", "Creating organization", "name", name)
102+
103+
if err := ValidateOrgName(name); err != nil {
69104
return nil, NewErrValidation(fmt.Errorf("invalid organization name: %w", err))
70105
}
71106

72-
return uc.orgRepo.Create(ctx, name)
107+
org, err := uc.orgRepo.Create(ctx, name)
108+
if err != nil {
109+
return nil, fmt.Errorf("failed to create organization: %w", err)
110+
}
111+
112+
return org, nil
73113
}
74114

75-
func validateOrgName(name string) error {
115+
func ValidateOrgName(name string) error {
76116
// The same validation done by Kubernetes for their namespace name
77117
// https://github.com/kubernetes/apimachinery/blob/fa98d6eaedb4caccd69fc07d90bbb6a1e551f00f/pkg/api/validation/generic.go#L63
78118
err := validation.IsDNS1123Label(name)
@@ -101,7 +141,7 @@ func (uc *OrganizationUseCase) Update(ctx context.Context, userID, orgID string,
101141

102142
// We validate the name to get ready for the name to become identifiers
103143
if name != nil {
104-
if err := validateOrgName(*name); err != nil {
144+
if err := ValidateOrgName(*name); err != nil {
105145
return nil, NewErrValidation(fmt.Errorf("invalid organization name: %w", err))
106146
}
107147
}

‎app/controlplane/internal/biz/organization_integration_test.go‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35,16 +35,22 @@ import (
3535
"github.com/stretchr/testify/suite"
3636
)
3737

38-
// and delete cascades that we want to validate that they work too
38+
func (s *OrgIntegrationTestSuite) TestCreateWithRandomName() {
39+
// It can create thousands of orgs without any problem
40+
for i := 0; i < 1000; i++ {
41+
org, err := s.Organization.CreateWithRandomName(context.Background())
42+
s.NoError(err)
43+
s.NotNil(org)
44+
}
45+
}
46+
3947
func (s *OrgIntegrationTestSuite) TestCreate() {
4048
ctx := context.Background()
4149

4250
testCases := []struct {
4351
name string
4452
expectedError bool
4553
}{
46-
// autogenerated name
47-
{"", false},
4854
{"a", false},
4955
{"aa-aa", false},
5056
{"-aaa", true},
@@ -96,7 +102,7 @@ func (s *OrgIntegrationTestSuite) TestUpdate() {
96102
})
97103

98104
s.T().Run("org not accessible to user", func(t *testing.T) {
99-
org2, err := s.Organization.Create(ctx, "testing-org")
105+
org2, err := s.Organization.CreateWithRandomName(ctx)
100106
require.NoError(s.T(), err)
101107
_, err = s.Organization.Update(ctx, s.user.ID, org2.ID, nil)
102108
s.Error(err)

‎app/controlplane/internal/biz/organization_test.go‎

Lines changed: 33 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,18 +13,49 @@
1313
// See the License for the specific language governing permissions and
1414
// limitations under the License.
1515

16-
package biz
16+
package biz_test
1717

1818
import (
19+
"context"
20+
"io"
1921
"testing"
2022

23+
"github.com/chainloop-dev/chainloop/app/controlplane/internal/biz"
24+
repoM "github.com/chainloop-dev/chainloop/app/controlplane/internal/biz/mocks"
25+
"github.com/go-kratos/kratos/v2/log"
26+
"github.com/stretchr/testify/mock"
2127
"github.com/stretchr/testify/suite"
2228
)
2329

2430
type organizationTestSuite struct {
2531
suite.Suite
2632
}
2733

34+
func (s *organizationTestSuite) TestCreateWithRandomName() {
35+
repo := repoM.NewOrganizationRepo(s.T())
36+
uc := biz.NewOrganizationUseCase(repo, nil, nil, nil, log.NewStdLogger(io.Discard))
37+
38+
s.Run("the org exists, we retry", func() {
39+
ctx := context.Background()
40+
// the first one fails because it already exists
41+
repo.On("Create", ctx, mock.Anything).Once().Return(nil, biz.ErrAlreadyExists)
42+
// but the second call creates the org
43+
repo.On("Create", ctx, mock.Anything).Once().Return(&biz.Organization{Name: "foobar"}, nil)
44+
got, err := uc.CreateWithRandomName(ctx)
45+
s.NoError(err)
46+
s.Equal("foobar", got.Name)
47+
})
48+
49+
s.Run("if it runs out of tries, it fails", func() {
50+
ctx := context.Background()
51+
// the first one fails because it already exists
52+
repo.On("Create", ctx, mock.Anything).Times(biz.OrganizationRandomNameMaxTries).Return(nil, biz.ErrAlreadyExists)
53+
got, err := uc.CreateWithRandomName(ctx)
54+
s.Error(err)
55+
s.Nil(got)
56+
})
57+
}
58+
2859
func (s *organizationTestSuite) TestValidateOrgName() {
2960
testCases := []struct {
3061
name string
@@ -47,7 +78,7 @@ func (s *organizationTestSuite) TestValidateOrgName() {
4778

4879
for _, tc := range testCases {
4980
s.T().Run(tc.name, func(t *testing.T) {
50-
err := validateOrgName(tc.name)
81+
err := biz.ValidateOrgName(tc.name)
5182
if tc.expectedError {
5283
s.Error(err)
5384
} else {

‎app/controlplane/internal/biz/referrer_integration_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -359,9 +359,9 @@ func (s *referrerIntegrationTestSuite) SetupTest() {
359359
ctx := context.Background()
360360

361361
var err error
362-
s.org1, err = s.Organization.Create(ctx, "testing-org")
362+
s.org1, err = s.Organization.CreateWithRandomName(ctx)
363363
require.NoError(s.T(), err)
364-
s.org2, err = s.Organization.Create(ctx, "testing-org-2")
364+
s.org2, err = s.Organization.CreateWithRandomName(ctx)
365365
require.NoError(s.T(), err)
366366

367367
s.org1UUID, err = uuid.Parse(s.org1.ID)

‎app/controlplane/internal/biz/user_integration_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -78,9 +78,9 @@ func (s *userIntegrationTestSuite) SetupTest() {
7878

7979
s.TestingUseCases = testhelpers.NewTestingUseCases(t)
8080

81-
s.userOneOrg, err = s.Organization.Create(ctx, "user-1-org")
81+
s.userOneOrg, err = s.Organization.CreateWithRandomName(ctx)
8282
assert.NoError(err)
83-
s.sharedOrg, err = s.Organization.Create(ctx, "shared-org")
83+
s.sharedOrg, err = s.Organization.CreateWithRandomName(ctx)
8484
assert.NoError(err)
8585

8686
// Create User 1

‎app/controlplane/internal/biz/workflow_integration_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ func (s *workflowIntegrationTestSuite) TestUpdate() {
3838
project = "test project"
3939
)
4040

41-
org2, err := s.Organization.Create(context.Background(), "testing-org")
41+
org2, err := s.Organization.CreateWithRandomName(context.Background())
4242
require.NoError(s.T(), err)
4343
workflow, err := s.Workflow.Create(ctx, &biz.WorkflowCreateOpts{Name: name, OrgID: s.org.ID})
4444
require.NoError(s.T(), err)
@@ -148,7 +148,7 @@ func (s *workflowIntegrationTestSuite) SetupTest() {
148148
s.TestingUseCases = testhelpers.NewTestingUseCases(s.T())
149149

150150
ctx := context.Background()
151-
s.org, err = s.Organization.Create(ctx, "testing-org")
151+
s.org, err = s.Organization.CreateWithRandomName(ctx)
152152
assert.NoError(err)
153153
}
154154

0 commit comments

Comments
 (0)