Merge pull request #13585 from wallyworld/explicit-offer-owner

#13585

The add offer api was using the authenticated user as the owner of the offer. However, this breaks on JAAS where the add api is always called as the admin user, and the offer owner needs to be passed in as a parameter. This PR adds the owner to the add offer params. The Juju client sets the value to the logged in user; JAAS is now able to set the owner to the relevant value also.

## QA steps

bootstrap a 2.9.22 controller
create an offer
consume the offer
repeat with a controller bootstrapped off this PR

## Bug reference

https://bugs.launchpad.net/juju/+bug/1952757

api/applicationoffers/client.go

@@ -32,7 +32,7 @@ func NewClient(st base.APICallCloser) *Client {
 }
 
 // Offer prepares application's endpoints for consumption.
-func (c *Client) Offer(modelUUID, application string, endpoints []string, offerName string, desc string) ([]params.ErrorResult, error) {
+func (c *Client) Offer(modelUUID, application string, endpoints []string, owner, offerName, desc string) ([]params.ErrorResult, error) {
 	// TODO(wallyworld) - support endpoint aliases
 	ep := make(map[string]string)
 	for _, name := range endpoints {
@@ -45,6 +45,7 @@ func (c *Client) Offer(modelUUID, application string, endpoints []string, offerN
 			ApplicationDescription: desc,
 			Endpoints:              ep,
 			OfferName:              offerName,
+			OwnerTag:               names.NewUserTag(owner).String(),
 		},
 	}
 	out := params.ErrorResults{}

api/applicationoffers/client_test.go

@@ -33,6 +33,7 @@ func (s *crossmodelMockSuite) TestOffer(c *gc.C) {
 	endPointB := "endPointB"
 	offer := "offer"
 	desc := "desc"
+	owner := "fred"
 
 	msg := "fail"
 	apiCaller := basetesting.APICallerFunc(
@@ -54,6 +55,7 @@ func (s *crossmodelMockSuite) TestOffer(c *gc.C) {
 			c.Assert(offer.ApplicationName, gc.Equals, application)
 			c.Assert(offer.Endpoints, jc.DeepEquals, map[string]string{endPointA: endPointA, endPointB: endPointB})
 			c.Assert(offer.OfferName, gc.Equals, offer.OfferName)
+			c.Assert(offer.OwnerTag, gc.Equals, "user-"+owner)
 			c.Assert(offer.ApplicationDescription, gc.Equals, desc)
 
 			if results, ok := result.(*params.ErrorResults); ok {
@@ -68,7 +70,7 @@ func (s *crossmodelMockSuite) TestOffer(c *gc.C) {
 		})
 
 	client := applicationoffers.NewClient(apiCaller)
-	results, err := client.Offer("uuid", application, []string{endPointA, endPointB}, offer, desc)
+	results, err := client.Offer("uuid", application, []string{endPointA, endPointB}, owner, offer, desc)
 	c.Assert(err, jc.ErrorIsNil)
 	c.Assert(results, gc.HasLen, 2)
 	c.Assert(results, jc.DeepEquals,
@@ -93,7 +95,7 @@ func (s *crossmodelMockSuite) TestOfferFacadeCallError(c *gc.C) {
 			return errors.New(msg)
 		})
 	client := applicationoffers.NewClient(apiCaller)
-	results, err := client.Offer("", "", nil, "", "")
+	results, err := client.Offer("", "", nil, "fred", "", "")
 	c.Assert(errors.Cause(err), gc.ErrorMatches, msg)
 	c.Assert(results, gc.IsNil)
 }

api/facadeversions.go

@@ -19,7 +19,7 @@ var facadeVersions = map[string]int{
 	"AllWatcher":                   1,
 	"Annotations":                  2,
 	"Application":                  13,
-	"ApplicationOffers":            3,
+	"ApplicationOffers":            4,
 	"ApplicationScaler":            1,
 	"Backups":                      2,
 	"Block":                        2,

apiserver/allfacades.go

@@ -167,7 +167,8 @@ func AllFacades() *facade.Registry {
 
 	reg("ApplicationOffers", 1, applicationoffers.NewOffersAPI)
 	reg("ApplicationOffers", 2, applicationoffers.NewOffersAPIV2)
-	reg("ApplicationOffers", 3, applicationoffers.NewOffersAPIV3) // Add user to consume offers details  args.
+	reg("ApplicationOffers", 3, applicationoffers.NewOffersAPIV3) // Add user to consume offers details args.
+	reg("ApplicationOffers", 4, applicationoffers.NewOffersAPIV4) // Add user to add offer args.
 	reg("ApplicationScaler", 1, applicationscaler.NewAPI)
 	reg("Backups", 1, backups.NewFacade)
 	reg("Backups", 2, backups.NewFacadeV2)

apiserver/facades/client/applicationoffers/applicationoffers.go

@@ -45,6 +45,11 @@ type OffersAPIV3 struct {
 	*OffersAPIV2
 }
 
+// OffersAPIV4 implements the cross model interface V4.
+type OffersAPIV4 struct {
+	*OffersAPIV3
+}
+
 // createAPI returns a new application offers OffersAPI facade.
 func createOffersAPI(
 	getApplicationOffers func(interface{}) jujucrossmodel.ApplicationOffers,
@@ -77,7 +82,7 @@ func createOffersAPI(
 	return api, nil
 }
 
-// NewOffersAPIV2 returns a new application offers OffersAPIV2 facade.
+// NewOffersAPI returns a new application offers OffersAPI facade.
 func NewOffersAPI(ctx facade.Context) (*OffersAPI, error) {
 	environFromModel := func(modelUUID string) (environs.Environ, error) {
 		st, err := ctx.StatePool().Get(modelUUID)
@@ -133,11 +138,20 @@ func NewOffersAPIV3(ctx facade.Context) (*OffersAPIV3, error) {
 	return &OffersAPIV3{OffersAPIV2: apiV2}, nil
 }
 
+// NewOffersAPIV4 returns a new application offers OffersAPIV4 facade.
+func NewOffersAPIV4(ctx facade.Context) (*OffersAPIV4, error) {
+	apiV3, err := NewOffersAPIV3(ctx)
+	if err != nil {
+		return nil, errors.Trace(err)
+	}
+	return &OffersAPIV4{OffersAPIV3: apiV3}, nil
+}
+
 // Offer makes application endpoints available for consumption at a specified URL.
 func (api *OffersAPI) Offer(all params.AddApplicationOffers) (params.ErrorResults, error) {
 	result := make([]params.ErrorResult, len(all.Offers))
 
-	user := api.Authorizer.GetAuthTag().(names.UserTag)
+	apiUser := api.Authorizer.GetAuthTag().(names.UserTag)
 	for i, one := range all.Offers {
 		modelTag, err := names.ParseModelTag(one.ModelTag)
 		if err != nil {
@@ -151,12 +165,21 @@ func (api *OffersAPI) Offer(all params.AddApplicationOffers) (params.ErrorResult
 		}
 		defer releaser()
 
-		if err := api.checkAdmin(user, backend); err != nil {
+		if err := api.checkAdmin(apiUser, backend); err != nil {
 			result[i].Error = apiservererrors.ServerError(err)
 			continue
 		}
 
-		applicationOfferParams, err := api.makeAddOfferArgsFromParams(user, backend, one)
+		owner := apiUser
+		// The V4 version of the api includes the offer owner in the params.
+		if one.OwnerTag != "" {
+			var err error
+			if owner, err = names.ParseUserTag(one.OwnerTag); err != nil {
+				result[i].Error = apiservererrors.ServerError(err)
+				continue
+			}
+		}
+		applicationOfferParams, err := api.makeAddOfferArgsFromParams(owner, backend, one)
 		if err != nil {
 			result[i].Error = apiservererrors.ServerError(err)
 			continue

apiserver/facades/client/applicationoffers/applicationoffers_test.go

@@ -75,13 +75,14 @@ func (s *applicationOffersSuite) assertOffer(c *gc.C, expectedErr error) {
 		OfferName:       "offer-test",
 		ApplicationName: applicationName,
 		Endpoints:       map[string]string{"db": "db"},
+		OwnerTag:        "user-fred",
 	}
 	all := params.AddApplicationOffers{Offers: []params.AddApplicationOffer{one}}
 	s.applicationOffers.addOffer = func(offer jujucrossmodel.AddApplicationOfferArgs) (*jujucrossmodel.ApplicationOffer, error) {
 		c.Assert(offer.OfferName, gc.Equals, one.OfferName)
 		c.Assert(offer.ApplicationName, gc.Equals, one.ApplicationName)
 		c.Assert(offer.ApplicationDescription, gc.Equals, "A pretty popular blog engine")
-		c.Assert(offer.Owner, gc.Equals, "admin")
+		c.Assert(offer.Owner, gc.Equals, "fred")
 		c.Assert(offer.HasRead, gc.DeepEquals, []string{"everyone@external"})
 		return &jujucrossmodel.ApplicationOffer{}, nil
 	}

apiserver/facades/schema.json

@@ -4321,8 +4321,8 @@
     },
     {
         "Name": "ApplicationOffers",
-        "Description": "OffersAPIV3 implements the cross model interface V3.",
-        "Version": 3,
+        "Description": "OffersAPIV4 implements the cross model interface V4.",
+        "Version": 4,
         "AvailableTo": [
             "controller-machine-agent",
             "machine-agent",
@@ -4452,6 +4452,9 @@
                         },
                         "offer-name": {
                             "type": "string"
+                        },
+                        "owner-tag": {
+                            "type": "string"
                         }
                     },
                     "additionalProperties": false,

apiserver/params/crossmodel.go

@@ -124,6 +124,7 @@ type AddApplicationOffer struct {
 	ApplicationName        string            `json:"application-name"`
 	ApplicationDescription string            `json:"application-description"`
 	Endpoints              map[string]string `json:"endpoints"`
+	OwnerTag               string            `json:"owner-tag,omitempty"`
 }
 
 // DestroyApplicationOffers holds parameters for the DestroyOffers call.

cmd/juju/application/deploy_test.go

@@ -869,6 +869,7 @@ func (s *DeploySuite) TestDeployBundleWithOffers(c *gc.C) {
 		"deadbeef-0bad-400d-8000-4b1d0d06f00d",
 		"apache2",
 		[]string{"apache-website", "website-cache"},
+		"admin",
 		"my-offer",
 		"",
 	).Returns([]params.ErrorResult{}, nil)
@@ -877,6 +878,7 @@ func (s *DeploySuite) TestDeployBundleWithOffers(c *gc.C) {
 		"deadbeef-0bad-400d-8000-4b1d0d06f00d",
 		"apache2",
 		[]string{"apache-website"},
+		"admin",
 		"my-other-offer",
 		"",
 	).Returns([]params.ErrorResult{}, nil)
@@ -2820,8 +2822,8 @@ func (f *fakeDeployAPI) ScaleApplication(p application.ScaleApplicationParams) (
 	}, nil
 }
 
-func (f *fakeDeployAPI) Offer(modelUUID, application string, endpoints []string, offerName, descr string) ([]params.ErrorResult, error) {
-	results := f.MethodCall(f, "Offer", modelUUID, application, endpoints, offerName, descr)
+func (f *fakeDeployAPI) Offer(modelUUID, application string, endpoints []string, owner, offerName, descr string) ([]params.ErrorResult, error) {
+	results := f.MethodCall(f, "Offer", modelUUID, application, endpoints, owner, offerName, descr)
 	return results[0].([]params.ErrorResult), jujutesting.TypeAssertError(results[1])
 }
 

cmd/juju/application/deployer/bundlehandler.go

@@ -1418,7 +1418,7 @@ func (h *bundleHandler) createOffer(change *bundlechanges.CreateOfferChange) err
 	}
 
 	p := change.Params
-	result, err := h.deployAPI.Offer(h.targetModelUUID, p.Application, p.Endpoints, p.OfferName, "")
+	result, err := h.deployAPI.Offer(h.targetModelUUID, p.Application, p.Endpoints, h.accountUser, p.OfferName, "")
 	if err == nil && len(result) > 0 && result[0].Error != nil {
 		err = result[0].Error
 	}

cmd/juju/application/deployer/interface.go

@@ -87,7 +87,7 @@ type CharmDeployAPI interface {
 // OfferAPI represents the methods of the API the deploy command needs
 // for creating offers.
 type OfferAPI interface {
-	Offer(modelUUID, application string, endpoints []string, offerName, descr string) ([]apiparams.ErrorResult, error)
+	Offer(modelUUID, application string, endpoints []string, owner, offerName, descr string) ([]apiparams.ErrorResult, error)
 	GrantOffer(user, access string, offerURLs ...string) error
 }