Skip to content

New service config model #128

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

Merged
merged 5 commits into from
Jun 23, 2017
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
158 changes: 155 additions & 3 deletions mixer/v1/config/cfg.proto
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,27 @@ package istio.mixer.v1.config;
// value: response.time
Copy link
Contributor

Choose a reason for hiding this comment

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

We should axe the outdated example here and stick with just showing the new config.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Who(what tools) is using this proto actively to understand the comments ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am fine with removing, just want to make sure to not delete currently useful data. I know adding more is also like a breaking change.

// labels:
// statusCode: response.code
//
// actionRules:
// - selector: target.service == "*"
// actions:
// - handler: prometheus-handler
// instances:
// - RequestCountByService
//
// handlers:
// - name: prometheus-handler
// adapter: prometheus
// params:
//
// constructors:
// - instanceName: RequestCountByService
Copy link
Contributor

Choose a reason for hiding this comment

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

instance_name

Copy link
Contributor

Choose a reason for hiding this comment

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

Camel case in YAML, snake case in protos; this is correct.

Copy link
Contributor

@geeknoid geeknoid Jun 20, 2017

Choose a reason for hiding this comment

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

OK smarty pants, then target_ip below should be targetIp. :-)

Copy link
Contributor

Choose a reason for hiding this comment

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

:P

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just to clarify.. in YAML both would work for a proto field, but Camel is recommended..
However, the target_ip is not a proto field, it is just a string (label name) so it can be anything, there is no guidelines on that. So you both are kind of right :-).

There are few more label names that has underscores, I can change them too if we want to make the label names to follow camelCase, let me know.

// template: istio.mixer.adapter.metric.Metric
// params:
// value: 1
// dimensions:
// source: origin.service
// target_ip: destination.ip
// ```
//
// (== deprecation_description ServiceConfig is deprecated, see the Config API's
Expand All @@ -55,7 +76,16 @@ message ServiceConfig {
string subject = 1;
// Optional. revision of this config. This is assigned by the server
string revision = 2;
repeated AspectRule rules = 3;
repeated AspectRule rules = 3 [deprecated = true];
Copy link
Contributor

Choose a reason for hiding this comment

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

@mandarjog can we just delete the ServiceConfig message? We don't need any of these wrapper messages given the new granular config API, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would request to leave this until the new granualar API is up and running. This field will kind of help in future PRs around the new Mixer Model codegen work (till we get the new granular API).


// Under development, DO NOT USE
// Optional. List of constructors that can be referenced from [actions][istio.mixer.v1.config.Action.instances]
repeated Constructor constructors = 4;

// Under development, DO NOT USE
// Optional. List of actions that apply for this service
// TODO: Rename this to rules once we delete the AspectRule field.
repeated Rule action_rules = 5;
}

// An AspectRule is a selector and a set of intentions to be executed when the
Expand All @@ -76,6 +106,7 @@ message AspectRule {
string selector = 1;
// The aspects that apply when selector evaluates to `true`.
repeated Aspect aspects = 2;

// Nested aspect rules; their selectors are evaluated if this selector
// predicate evaluates to `true`.
repeated AspectRule rules = 3;
Expand Down Expand Up @@ -127,7 +158,7 @@ message Aspect {
// params:
// ```
message Adapter {
// Required, must be unique per `kind`. Used by [Aspect][istio.mixer.v1.config.Aspect]
// Required. must be unique per `kind`. Used by [Aspect][istio.mixer.v1.config.Aspect]
// to refer to this adapter. The name "default" is special: when an Aspect does not
// specify a name, the Adapter named "default" of the same `kind` is used to execute
// the intention described by the [AspectRule][istio.mixer.v1.config.AspectRule]s.
Expand All @@ -139,7 +170,7 @@ message Adapter {
// Required. The name of a specific adapter implementation. An adapter's
// implementation name is typically a constant in its code.
string impl = 3;
// Optional, depends on adapter implementation. Struct representation of a
// Optional. depends on adapter implementation. Struct representation of a
// proto defined by the implementation; this varies depending on `impl`.
google.protobuf.Struct params = 4;
}
Expand All @@ -165,6 +196,10 @@ message GlobalConfig {
repeated istio.mixer.v1.config.descriptor.MonitoredResourceDescriptor monitored_resources = 6;
repeated istio.mixer.v1.config.descriptor.PrincipalDescriptor principals = 7;
repeated istio.mixer.v1.config.descriptor.QuotaDescriptor quotas = 8;

// Under development, DO NOT USE
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we just not add this field yet?

Also, @mandarjog same question as ServiceConfig: can this message just be deleted?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This field will kind of help in future PRs (till we get the new granular API).

// Optional. List of handlers that can be referenced from [actions][istio.mixer.v1.config.Action.handler]
repeated Handler handlers = 9;
}

// AttributeManifest describes a set of Attributes produced by some component
Expand Down Expand Up @@ -260,3 +295,120 @@ message DnsName {
message EmailAddress {
string value = 1;
}


// A Rule is a selector and a set of intentions to be executed when the
// selector is `true`
//
// The following example instructs Mixer to invoke 'prometheus-handler' handler for all services and pass it the
// instance constructed using the 'RequestCountByService' constructor.
//
// ```yaml
// actionRules:
// - selector: target.service == "*"
// actions:
// - handler: prometheus-handler
// instances:
// - RequestCountByService
// ```
message Rule {
// Required. Selector is an attribute based predicate. When Mixer receives a
// request it evaluates all selectors in scope and executes the `actions` for all
// selectors that evaluated to true.
//
// A few example selectors:
//
// * an empty selector evaluates to `true`
// * `true`, a boolean literal; a rule with this selector will always be executed
// * `target.service == ratings*` selects any request targeting a service whose
// name starts with "ratings"
// * `attr1 == "20" && attr2 == "30"` logical AND, OR, and NOT are also available
string selector = 1;

// Optional. The actions that will be executed when selector evaluates to `true`.
repeated Action actions = 2;

// Optional. Nested action rules; their selectors are evaluated if this selector
// predicate evaluates to `true`.
repeated Rule rules = 3;
}

// Action describes which [Handler][istio.mixer.v1.config.Handler] to invoke and what data to pass to it for processing.
//
// The following example instructs Mixer to invoke 'prometheus-handler' handler and pass it the object
// constructed using the constructor 'RequestCountByService'
//
// ```yaml
// - handler: prometheus-handler
// instances:
// - RequestCountByService
// ```
message Action {
// Required. The handler to invoke. Must match the `name` of a [Handler][istio.mixer.v1.config.Handler.name] in scope.
string handler = 2;

// Required. Each value must match the `instance_name` of the
// [Constructor][istio.mixer.v1.config.Constructor.instance_name]s in scope.
// Referenced constructors are evaluated by resolving the attributes/literals for all the fields.
// The constructed objects are then passed to the `handler` referenced within this action.
repeated string instances = 3;
}

// A Constructor tells Mixer how to create instances for particular template.
//
// Constructor is defined by the operator. Constructor is defined relative to a known
// template. Their purpose is to tell Mixer how to use attributes or literals to produce
// instances of the specified template at runtime.
//
// The following example instructs Mixer to construct an instance associated with template
// 'istio.mixer.adapter.metric.Metric'. It provides a mapping from the template's fields to expressions.
// Instances produced with this constructor can be referenced by [Actions][istio.mixer.v1.config.Action] using name
// 'RequestCountByService'.
//
// ```yaml
// - instanceName: RequestCountByService
// template: istio.mixer.adapter.metric.Metric
// params:
// value: 1
// dimensions:
// source: origin.service
// target_ip: destination.ip
// ```
message Constructor {
// Required. The name of this constructor
Copy link
Contributor

Choose a reason for hiding this comment

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

Names of instances produced by this constructor. Constructors are currently nameless.

//
// Must be unique amongst other Constructors in scope. Used by [Action][istio.mixer.v1.config.Action] to refer
Copy link
Contributor

Choose a reason for hiding this comment

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

other Constructors -> other Instances.

// to an instance produced by this constructor.
string instance_name = 1;

// Required. The name of the template this constructor creates instances for.
// The value must match the name of the available template in scope.
string template = 2;
Copy link
Contributor

Choose a reason for hiding this comment

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

We use instance_name above, so should this be template_name to be consistent?


// Required. Depends on referenced template. Struct representation of a
// proto defined by the template; this varies depending on the value of field `template`.
google.protobuf.Struct params = 3;
}

// Handler allows the operator to configure a specific adapter implementation.
// Each adapter implementation defines its own `params` proto.
//
// In the following example we define a `metrics` adapter using the Mixer's prepackaged
Copy link
Contributor

Choose a reason for hiding this comment

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

adapter -> handler

// prometheus adapter. This adapter doesn't require any parameters.
Copy link
Contributor

Choose a reason for hiding this comment

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

This adapter -> this handler

//
// ```yaml
// name: prometheus-handler
// adapter: prometheus
// params:
// ```
message Handler {
// Required. Must be unique in the entire mixer configuration. Used by [Actions][istio.mixer.v1.config.Action.handler]
// to refer to this handler.
string name = 1;
// Required. The name of a specific adapter implementation. An adapter's
// implementation name is typically a constant in its code.
string adapter = 2;
// Optional. Depends on adapter implementation. Struct representation of a
// proto defined by the adapter implementation; this varies depending on the value of field `adapter`.
google.protobuf.Struct params = 3;
}