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

Display value differences to actual environment #158

Merged
merged 3 commits into from
Jun 15, 2023
Merged
Show file tree
Hide file tree
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
1 change: 1 addition & 0 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ gem 'net-ldap', require: "net/ldap"
gem 'breadcrumbs_on_rails'
gem 'cancancan'
gem 'ruby-saml'
gem 'diffy'

# To use retry middleware with Faraday v2.0+
gem 'faraday-retry'
Expand Down
2 changes: 2 additions & 0 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ GEM
railties (>= 6.0.0)
date (3.3.3)
deep_merge (1.2.2)
diffy (3.4.2)
docile (1.4.0)
erubi (1.12.0)
execjs (2.8.1)
Expand Down Expand Up @@ -423,6 +424,7 @@ DEPENDENCIES
cancancan
capybara (>= 2.15)
dartsass-rails
diffy
factory_bot_rails
faker
faraday-retry
Expand Down
15 changes: 15 additions & 0 deletions app/assets/stylesheets/application.scss
Original file line number Diff line number Diff line change
Expand Up @@ -41,3 +41,18 @@ ol.breadcrumb li:first-child {
width: 1em;
height: 1em;
}

// Diffy Styles
.diff{overflow:auto;}
.diff ul{background:#fff;overflow:auto;font-size:13px;list-style:none;margin:0;padding:0;display:table;width:100%;}
.diff del, .diff ins{display:block;text-decoration:none;}
.diff li{padding:0; display:table-row;margin: 0;height:1em;}
.diff li.ins{background:#dfd; color:#080}
.diff li.del{background:#fee; color:#b00}
.diff li:hover{background:#ffc}
/* try 'whitespace:pre;' if you don't want lines to wrap */
.diff del, .diff ins, .diff span{white-space:pre-wrap;font-family:courier;}
.diff del strong{font-weight:normal;background:#fcc;}
.diff ins strong{font-weight:normal;background:#9f9;}
.diff li.diff-comment { display: none; }
.diff li.diff-block-info { background: none repeat scroll 0 0 gray; }
29 changes: 29 additions & 0 deletions app/models/data_file.rb
Original file line number Diff line number Diff line change
Expand Up @@ -66,4 +66,33 @@ def replaced_from_git?
def to_param
path
end

def id
[node&.name, path].join("-").parameterize
end

def has_differing_value_in_original_environment?(key)
return false unless environment && node.environment
return false if environment == node.environment

candidate_files = node.environment.hierarchies.map do |h|
self.class.new(hierarchy: h, path:, node:)
end
other_file = candidate_files.find(&:exist?)
return false unless other_file

other_key = Key.new(environment: node.environment, name: key.name)
return false if value_for(key:).value == other_file.value_for(key: other_key).value

true
end

def value_from_original_environment(key:)
candidate_files = (node&.environment&.hierarchies || []).map do |h|
self.class.new(hierarchy: h, path:, node:)
end
other_file = candidate_files.find(&:exist?)
other_key = Key.new(environment: node.environment, name: key.name)
other_file.value_for(key: other_key)
end
end
15 changes: 15 additions & 0 deletions app/views/keys/_diff.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
<% split_diff = Diffy::SplitDiff.new(file.value_from_original_environment(key: @key).value, value.value, format: :html) %>
<table class="table">
<thead>
<tr>
<th><%= @node.environment.name %></th>
<th><%= @environment.name %></th>
</tr>
</thead>
<tbody>
<tr>
<td><%= raw(split_diff.left) %></td>
<td><%= raw(split_diff.right) %></td>
</tr>
</tbody>
</table>
22 changes: 22 additions & 0 deletions app/views/keys/_value.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
<% if file.has_differing_value_in_original_environment?(@key) %>
<ul class="nav nav-tabs mb-2" role="tablist">
<li class="nav-item" role="presentation">
<button id="<%= "tab-value-#{dom_id(file)}" %>" class="nav-link active" data-target="<%= "#value-#{dom_id(file)}" %>" data-toggle="tab" type="button" role="tab" aria-controls="<%= "value-#{dom_id(file)}" %>" aria-selected="true">Value</button>
</li>
<li class="nav-item" role="presentation">
<button id="<%= "tab-diff-#{dom_id(file)}" %>" class="nav-link" data-target="<%= "#diff-#{dom_id(file)}" %>" data-toggle="tab" type="button" role="tab" aria-controls="<%= "diff-#{dom_id(file)}" %>" aria-selected="false">Diff</button>
</li>
</ul>

<div class="tab-content">
<div id="<%= "value-#{dom_id(file)}" %>" class="tab-pane fade show active" role="tabpanel" aria-labelledby="<%= "tab-value-#{dom_id(file)}" %>">
<%= render "form", file: file, value: value %>
</div>

<div id="<%= "diff-#{dom_id(file)}" %>" class="tab-pane fade" role="tabpanel" aria-labelledby="<%= "tab-diff-#{dom_id(file)}" %>">
<%= render "diff", file: file, value: value %>
</div>
</div>
<% else %>
<%= render "form", file: file, value: value %>
<% end %>
2 changes: 1 addition & 1 deletion app/views/keys/show.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@

<div id="collapse-<%= index %>" class="collapse" aria-labelledby="path-<%= index %>">
<div class="card-body">
<%= render "form", file: file, value: value %>
<%= render "value", file: file, value: value %>
</div>
</div>
</div>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
---
foobar::firstrun::linux_classes:
hostname: hostname-role
hdm::integer: 1
hdm::float: 2.3
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
---
foobar::firstrun::linux_classes:
hostname: hostname-role
hdm::integer: 1
hdm::float: 4.5
4 changes: 2 additions & 2 deletions test/integration/key_search_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,11 @@ class KeySearchTest < ActionDispatch::IntegrationTest
sign_in_as(@user)
end

test "user withouth groups can find key" do
test "user without groups can find key" do
get environment_key_files_path(environment_id: "development", key_id: "hdm::integer")

assert_response :success
assert_select "p.lead", /Found\s+key\s+hdm::integer\s+in\s+1\s+file./
assert_select "p.lead", /Found\s+key\s+hdm::integer\s+in\s+2\s+files./
end

test "user with group that forbids access cannot find key" do
Expand Down
58 changes: 58 additions & 0 deletions test/models/data_file_test.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
require 'test_helper'

class DataFileTest < ActiveSupport::TestCase
setup do
@original_environment = Environment.new(name: "development")
@node = Node.new(environment: @original_environment, hostname: "test.host")
@globs_environment = Environment.new(name: "globs")
@path = "role/hdm_test.yaml"
end

test "#has_differing_value_in_original_environment? returns false if environments are the same" do
hierarchy = Hierarchy.find(@original_environment, "Eyaml hierarchy")
data_file = DataFile.new(hierarchy:, node: @node, path: @path)
key = Key.new(environment: @original_environment, name: "hdm::integer")

assert_not data_file.has_differing_value_in_original_environment?(key)
end

test "#has_differing_value_in_original_environment? returns false if original environment has no file with the same path" do
hierarchy = Hierarchy.find(@globs_environment, "Common")
data_file = DataFile.new(hierarchy:, node: @node, path: "common/foobar.yaml")
key = Key.new(environment: @globs_environment, name: "hdm::integer")

assert_not data_file.has_differing_value_in_original_environment?(key)
end

test "#has_differing_value_in_original_environment? returns false if original environment's value is the same" do
hierarchy = Hierarchy.find(@globs_environment, "Eyaml hierarchy")
data_file = DataFile.new(hierarchy:, node: @node, path: @path)
key = Key.new(environment: @globs_environment, name: "hdm::integer")

assert_not data_file.has_differing_value_in_original_environment?(key)
end

test "#has_differing_value_in_original_environment? returns true if original environment's value is different" do
hierarchy = Hierarchy.find(@globs_environment, "Eyaml hierarchy")
data_file = DataFile.new(hierarchy:, node: @node, path: @path)
key = Key.new(environment: @globs_environment, name: "hdm::float")

assert data_file.has_differing_value_in_original_environment?(key)
end

test "#value_for returns matching value from given environment" do
hierarchy = Hierarchy.find(@globs_environment, "Eyaml hierarchy")
data_file = DataFile.new(hierarchy:, node: @node, path: @path)
key = Key.new(environment: @globs_environment, name: "hdm::float")

assert_equal "4.5", data_file.value_for(key:).value
end

test "#value_from_original_environment returns matching value from the node's original environment" do
hierarchy = Hierarchy.find(@globs_environment, "Eyaml hierarchy")
data_file = DataFile.new(hierarchy:, node: @node, path: @path)
key = Key.new(environment: @globs_environment, name: "hdm::float")

assert_equal "2.3", data_file.value_from_original_environment(key:).value
end
end