-
Notifications
You must be signed in to change notification settings - Fork 25
Add CI-ready entitlement smart diff #74
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
Changes from all commits
3714891
067c29d
f4bceb0
b604b47
a89b2d7
39dee2d
abdb41c
540a75a
f843155
97222bf
fcb3f5f
2c7cccb
23d7128
f2e5507
c07c790
552fac0
0375819
4f855bf
cf72709
ee7ad61
3f50b77
d0fb95c
5e8c328
567cf48
78f8504
8102f0e
09c2e8f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| #!/usr/bin/env ruby | ||
|
|
||
| require "entitlements" | ||
| require "entitlements/smart_diff/cli" | ||
|
|
||
| exit Entitlements::SmartDiff::Cli.run |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -118,7 +118,7 @@ def filtered_members | |
| filters.reject { |_, filter_val| filter_val == :all }.each do |filter_name, filter_val| | ||
| filter_cfg = Entitlements::Data::Groups::Calculated.filters_index[filter_name] | ||
| clazz = filter_cfg.fetch(:class) | ||
| obj = clazz.new(filter: filter_val, config: filter_cfg.fetch(:config, {})) | ||
| obj = clazz.new(filter: filter_val, config: filter_cfg.fetch(:config, {}), options: options) | ||
| # If excluded_paths is set, ignore any of those excluded paths | ||
| unless filter_cfg[:config]["excluded_paths"].nil? | ||
| # if the filename is not in any of the excluded paths, filter it | ||
|
|
@@ -217,7 +217,7 @@ def expired?(expiration, context) | |
| return false if expiration.nil? || expiration.strip.empty? | ||
| if expiration =~ /\A(\d{4})-(\d{2})-(\d{2})\z/ | ||
| year, month, day = Regexp.last_match(1).to_i, Regexp.last_match(2).to_i, Regexp.last_match(3).to_i | ||
| return Time.utc(year, month, day, 0, 0, 0) <= Time.now.utc | ||
| return Time.utc(year, month, day, 0, 0, 0) <= Entitlements.evaluation_time.utc | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can hit bugs by repeatedly calling |
||
| end | ||
| message = "Invalid expiration date #{expiration.inspect} in #{context} (expected format: YYYY-MM-DD)" | ||
| raise ArgumentError, message | ||
|
|
@@ -340,7 +340,7 @@ def handle_or(rule) | |
| # Returns C::SetOf[Entitlements::Models::Person] from a recursive call. | ||
| def handle_and(rule) | ||
| ensure_type!("and", rule, Array) | ||
| return result unless rule.any? | ||
| return Set.new unless rule.any? | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
|
|
||
| first_rule = rule.shift | ||
| ensure_type!("and", first_rule, Hash) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,10 +21,12 @@ class Text < Entitlements::Data::Groups::Calculated::Base | |
| # Returns a Set[String] with DN's of the people in the group. | ||
| Contract C::None => C::Or[:calculating, C::SetOf[Entitlements::Models::Person]] | ||
| def members | ||
| @members ||= begin | ||
| Entitlements.logger.debug "Calculating members from #{filename}" | ||
| members_from_rules(rules) | ||
| end | ||
| return @members if @members | ||
|
|
||
| Entitlements.logger.debug "Calculating members from #{filename}" | ||
| result = members_from_rules(rules) | ||
| @members = result unless result == :calculating | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is both a general evaluator bug fix and a requirement for smart diff:
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nice, I was about to ask about the logic update. Make sense |
||
| result | ||
| end | ||
|
|
||
| # Standard interface: Get the description of this group. | ||
|
|
@@ -182,7 +184,7 @@ def rules | |
| if parsed_data.key?("modifier_expiration") && affirmative.empty? | ||
| exp_date = parsed_data.fetch("modifier_expiration").fetch("=").first.fetch(:key) | ||
| date = Entitlements::Util::Util.parse_date(exp_date) | ||
| return {"always" => false} if date <= Time.now.utc.to_date | ||
| return {"always" => false} if date <= Entitlements.evaluation_time.utc.to_date | ||
| end | ||
|
|
||
| # There has to be at least one affirmative condition, not just all negative ones. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,10 +18,12 @@ class YAML < Entitlements::Data::Groups::Calculated::Base | |
| # Returns a Set[String] with DN's of the people in the group. | ||
| Contract C::None => C::Or[:calculating, C::SetOf[Entitlements::Models::Person]] | ||
| def members | ||
| @members ||= begin | ||
| Entitlements.logger.debug "Calculating members from #{filename}" | ||
| members_from_rules(rules) | ||
| end | ||
| return @members if @members | ||
|
|
||
| Entitlements.logger.debug "Calculating members from #{filename}" | ||
| result = members_from_rules(rules) | ||
| @members = result unless result == :calculating | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is both a general evaluator bug fix and a requirement for smart diff: |
||
| result | ||
| end | ||
|
|
||
| # Standard interface: Get the description of this group. | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would like to know your thinking around shipping this in this repo instead of in github/entitlements.
We don't really have any concept of 'CI' or 'diff' in here. But we have been using
noopdeploys to compute the effective diff, which gives us the "real diff" because it also talks to all the downstream systems.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm ambivalent to which repo it ships in. We could make the 2-3 changes that are needed to the entitlements core (which are also bugfixes), and then move the bulk of this code to our private configuration repo and I'd be OK with that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would prefer that yeah