Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add product entitlement check for Origin Inspector before enabling the subscriber for origins. #118

Merged
merged 12 commits into from
Dec 1, 2022

Conversation

leklund
Copy link
Member

@leklund leklund commented Nov 16, 2022

If origin inspector has not been enabled all requests for real time origin metrics will no data. This PR checks for the product before calling subscriber.RunOrigins.

It will not update once the exporter is running so when origin inspector has been purchased or enabled at the customer level, the exporter will need to be restarted.

cmd/fastly-exporter/main.go Outdated Show resolved Hide resolved

level.Debug(p.logger).Log("product", response.Name, "hasAccess", response.HasAccess)

p.products[response.Name] = response.HasAccess

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't ever need to list the full set of products from the api response, right? If not then we don't care about the products we don't have access to. Maybe we should switch products to be map[string]struct{}. If the product key is in the map then we have access. We can rename Products() to HasAccess(p string) bool. If we do this then we can pass a ProductCache pointer directly to the manager and the runOrigins() func inside the manager goes away since it can just call HasAccess() directly where it's currently calling runOrigins().

@peterbourgon
Copy link
Contributor

peterbourgon commented Nov 17, 2022

If you call Refresh only at program start and never again, it's not really a cache, it's just a value, you'd be better served by just fetching the values and providing them as a slice of strings or whatever to the service. If it's a cache then you should Refresh it on some schedule, and in that case it needs synchronization with an e.g. mutex.

pkg/rt/manager.go Outdated Show resolved Hide resolved
pkg/rt/manager.go Outdated Show resolved Hide resolved
pkg/api/product_cache.go Outdated Show resolved Hide resolved
pkg/api/product_cache.go Outdated Show resolved Hide resolved
pkg/api/product_cache.go Outdated Show resolved Hide resolved
pkg/rt/manager.go Outdated Show resolved Hide resolved
pkg/rt/manager.go Outdated Show resolved Hide resolved
pkg/rt/manager.go Outdated Show resolved Hide resolved
pkg/rt/manager.go Outdated Show resolved Hide resolved
delete(m.managed, id)
level.Debug(m.logger).Log("service_id", id, "interrupt", err)
}
for key := range m.managed {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you change this line to for key, irq := range m.managed then you don't need the lookup on line 105.

@leklund leklund merged commit d4ec0f6 into main Dec 1, 2022
@leklund leklund deleted the product_check branch December 1, 2022 16:48
Comment on lines +96 to +108
// HasAccess takes a product as a string and returns a boolean
// based on the response from the Product API.
func (p *ProductCache) HasAccess(product string) bool {
if product == Default {
return true
}
p.mtx.Lock()
defer p.mtx.Unlock()
if v, ok := p.products[product]; ok {
return v
}
return true
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will p.HasAccess("abcxyz") return true?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More generally is access to a given product default-allow?

Comment on lines +32 to +35
type subscriberKey struct {
serviceID string
product string
}
Copy link
Contributor

@peterbourgon peterbourgon Dec 3, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If product should be part of the subscriber key like this says, that would mean there would be different subscriber types for each product, e.g. a subscriberKey with serviceID=X and product=foo would represent an e.g. fooSubscriber, and a subscriberKey with serviceID=X and product=bar would map to an e.g. barSubscriber. But there's only a single subscriber type, it's parameterized on service ID, and it has methods for both (all) of the product features. That means "enabled products" is a property of that subscriber type, not the key. So this doesn't really work.

If you want distinct subscribers for unique service ID + product combinations, then product needs to be part of both the subscriberKey and a field in the subscriber struct, which is consumed by all of the methods on that struct that interact with the API. But if you don't put product in the subscriber struct, then you can't make product part of the subscriber key, and you have to manage products at a layer above individual subscribers.

Comment on lines +14 to +19
const (
// Default is the standard real-time stats available to all services
Default = "default"
// OriginInspector is the product name used to determine access to Origin Inspector via the entitlement API
OriginInspector = "origin_inspector"
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These identifiers resolve to api.Default and api.OriginInspector which are somewhat ambiguous.

const (
	// ProductDefault represents the standard real-time stats available to all services.
	ProductDefault = "default"

	// ProductOriginInspector represents the origin inspector stats available via the
	// entitlement API.
	ProductOriginInspector = "origin_inspector"
)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This much clearer, thank you.

Comment on lines +24 to +30
// Product models the response from the Fastly Product Entitlement API.
type Product struct {
HasAccess bool `json:"has_access"`
Meta struct {
Name string `json:"id"`
} `json:"product"`
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

//  Product models the response from the Fastly Product Entitlement API.
 type Product struct {
 	HasAccess bool `json:"has_access"`
 	Product   struct {
 		ID string `json:"id"`
 	} `json:"product"`
 }

Comment on lines +20 to +23
// ProductCache represents the api.ProductCache behavior.
type ProductCache interface {
HasAccess(string) bool
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

// HasAccesser models the read side of an api.ProductCache.
type HasAccesser interface {
	HasAccess(string) bool
}

@leklund leklund mentioned this pull request Dec 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants