diff --git a/internal/db/applicationoffer_test.go b/internal/db/applicationoffer_test.go index d0c7c6a9e..7b2bf9089 100644 --- a/internal/db/applicationoffer_test.go +++ b/internal/db/applicationoffer_test.go @@ -231,6 +231,7 @@ func (s *dbSuite) TestFindApplicationOffers(c *qt.C) { Name: "offer-1", ModelID: env.model.ID, ApplicationName: "app-1", + URL: "url-1", ApplicationDescription: "this is a test application description", Endpoints: []dbmodel.ApplicationOfferRemoteEndpoint{{ Name: "test-endpoint-1", @@ -249,6 +250,7 @@ func (s *dbSuite) TestFindApplicationOffers(c *qt.C) { UUID: "00000000-0000-0000-0000-000000000002", Name: "offer-2", ModelID: env.model.ID, + URL: "url-2", ApplicationName: "app-1", ApplicationDescription: "this is another test offer", Endpoints: []dbmodel.ApplicationOfferRemoteEndpoint{{ @@ -264,6 +266,7 @@ func (s *dbSuite) TestFindApplicationOffers(c *qt.C) { UUID: "00000000-0000-0000-0000-000000000003", Name: "test-3", ModelID: env.model.ID, + URL: "url-3", ApplicationName: "app-1", ApplicationDescription: "this is yet another application offer", Endpoints: []dbmodel.ApplicationOfferRemoteEndpoint{{ diff --git a/internal/dbmodel/applicationoffer_test.go b/internal/dbmodel/applicationoffer_test.go index 1a4f80863..45715f09e 100644 --- a/internal/dbmodel/applicationoffer_test.go +++ b/internal/dbmodel/applicationoffer_test.go @@ -3,9 +3,11 @@ package dbmodel_test import ( + "database/sql" "testing" qt "github.com/frankban/quicktest" + "github.com/juju/juju/state" "github.com/juju/names/v5" "github.com/canonical/jimm/internal/dbmodel" @@ -25,3 +27,38 @@ func TestApplicationOfferTag(t *testing.T) { ao2.SetTag(tag.(names.ApplicationOfferTag)) c.Check(ao2, qt.DeepEquals, ao) } + +func TestApplicationOfferUniqueConstraint(t *testing.T) { + c := qt.New(t) + db := gormDB(c) + cl, cred, ctl, u := initModelEnv(c, db) + + m := dbmodel.Model{ + Name: "test-model", + UUID: sql.NullString{ + String: "00000001-0000-0000-0000-0000-000000000001", + Valid: true, + }, + Owner: u, + Controller: ctl, + CloudRegion: cl.Regions[0], + CloudCredential: cred, + Type: "iaas", + IsController: false, + DefaultSeries: "warty", + Life: state.Alive.String(), + } + c.Assert(db.Create(&m).Error, qt.IsNil) + + ao := dbmodel.ApplicationOffer{ + Name: "offer1", + UUID: "00000003-0000-0000-0000-0000-000000000001", + URL: "foo", + Model: m, + } + c.Assert(db.Create(&ao).Error, qt.IsNil) + ao.ID = 0 + ao.Name = "offer2" + ao.UUID = "00000003-0000-0000-0000-0000-000000000002" + c.Assert(db.Create(&ao).Error, qt.ErrorMatches, `ERROR: duplicate key value violates unique constraint "application_offers_url_key" .*`) +} diff --git a/internal/dbmodel/sql/postgres/1_11.sql b/internal/dbmodel/sql/postgres/1_11.sql new file mode 100644 index 000000000..a60d59efe --- /dev/null +++ b/internal/dbmodel/sql/postgres/1_11.sql @@ -0,0 +1,4 @@ +-- 1_11.sql is a migration that enforces uniqueness on URLs in application offers. +ALTER TABLE application_offers ADD UNIQUE (url); + +UPDATE versions SET major=1, minor=11 WHERE component='jimmdb'; diff --git a/internal/dbmodel/version.go b/internal/dbmodel/version.go index f5229fc18..cecc92215 100644 --- a/internal/dbmodel/version.go +++ b/internal/dbmodel/version.go @@ -20,7 +20,7 @@ const ( // Minor is the minor version of the model described in the dbmodel // package. It should be incremented for any change made to the // database model from database model in a released JIMM. - Minor = 10 + Minor = 11 ) type Version struct { diff --git a/internal/jimm/applicationoffer.go b/internal/jimm/applicationoffer.go index d9189b824..7aa4107b9 100644 --- a/internal/jimm/applicationoffer.go +++ b/internal/jimm/applicationoffer.go @@ -5,6 +5,7 @@ package jimm import ( "context" "database/sql" + "fmt" "sort" "strings" @@ -68,6 +69,19 @@ func (j *JIMM) Offer(ctx context.Context, user *openfga.User, offer AddApplicati ApplicationName: offer.OfferName, } + // Verify offer URL doesn't already exist. + var offerCheck dbmodel.ApplicationOffer + offerCheck.URL = offerURL.String() + err = j.Database.GetApplicationOffer(ctx, &offerCheck) + if err == nil { + return errors.E(fmt.Sprintf("offer %s already exists, use a different name", offerURL.String()), errors.CodeAlreadyExists) + } else { + if errors.ErrorCode(err) != errors.CodeNotFound { + // Anything besides Not Found is a problem. + return errors.E(op, err) + } + } + api, err := j.dial(ctx, &model.Controller, names.ModelTag{}) if err != nil { return errors.E(op, err) @@ -108,10 +122,6 @@ func (j *JIMM) Offer(ctx context.Context, user *openfga.User, offer AddApplicati var doc dbmodel.ApplicationOffer doc.FromJujuApplicationOfferAdminDetailsV5(offerDetails) - if err != nil { - zapctx.Error(ctx, "failed to convert application offer details", zaputil.Error(err)) - return errors.E(op, err) - } doc.ModelID = model.ID err = j.Database.Transaction(func(db *db.Database) error { if err := db.AddApplicationOffer(ctx, &doc); err != nil { diff --git a/internal/jujuapi/applicationoffers_test.go b/internal/jujuapi/applicationoffers_test.go index 41f305eed..d63ca91cb 100644 --- a/internal/jujuapi/applicationoffers_test.go +++ b/internal/jujuapi/applicationoffers_test.go @@ -67,7 +67,7 @@ func (s *applicationOffersSuite) TestOffer(c *gc.C) { c.Assert(results, gc.HasLen, 1) c.Assert(results[0].Error, gc.Equals, (*jujuparams.Error)(nil)) - results, err = client.Offer(s.Model.UUID.String, "no-such-app", []string{s.endpoint.Name}, "bob@canonical.com", "test-offer", "test offer description") + results, err = client.Offer(s.Model.UUID.String, "no-such-app", []string{s.endpoint.Name}, "bob@canonical.com", "test-offer-foo", "test offer description") c.Assert(err, gc.Equals, nil) c.Assert(results, gc.HasLen, 1) c.Assert(results[0].Error, gc.Not(gc.IsNil)) @@ -83,6 +83,29 @@ func (s *applicationOffersSuite) TestOffer(c *gc.C) { c.Assert(results[0].Error.Code, gc.Equals, "unauthorized access") } +func (s *applicationOffersSuite) TestCreateMultipleOffersForSameApp(c *gc.C) { + conn := s.open(c, nil, "bob@canonical.com") + defer conn.Close() + client := applicationoffers.NewClient(conn) + + results, err := client.Offer(s.Model.UUID.String, "test-app", []string{s.endpoint.Name}, "bob@canonical.com", "test-offer", "test offer description") + c.Assert(err, gc.Equals, nil) + c.Assert(results, gc.HasLen, 1) + c.Assert(results[0].Error, gc.Equals, (*jujuparams.Error)(nil)) + + // Creating an offer with the same name as above. + results, err = client.Offer(s.Model.UUID.String, "test-app", []string{s.endpoint.Name}, "bob@canonical.com", "test-offer", "test offer description") + c.Assert(err, gc.Equals, nil) + c.Assert(results, gc.HasLen, 1) + c.Assert(results[0].Error, gc.ErrorMatches, `offer bob@canonical.com/model-1.test-offer already exists, use a different name.*`) + + // Creating an offer with a new name. + results, err = client.Offer(s.Model.UUID.String, "test-app", []string{s.endpoint.Name}, "bob@canonical.com", "test-offer-foo", "test offer description") + c.Assert(err, gc.Equals, nil) + c.Assert(results, gc.HasLen, 1) + c.Assert(results[0].Error, gc.Equals, (*jujuparams.Error)(nil)) +} + func (s *applicationOffersSuite) TestGetConsumeDetails(c *gc.C) { conn := s.open(c, nil, "bob@canonical.com") defer conn.Close()