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

Bc regex bug #617

Merged
merged 2 commits into from
Jul 31, 2020
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
18 changes: 12 additions & 6 deletions apps/dashboard/app/models/batch_connect/app.rb
Original file line number Diff line number Diff line change
Expand Up @@ -119,20 +119,26 @@ def link
def configured_clusters
Array.wrap(form_config.fetch(:cluster, nil))
.select { |c| !c.to_s.strip.empty? }
.map { |c| c.to_s.strip.eql?("*") ? ".*" : c.to_s.strip }
.map { |c| c.to_s.strip }
.compact
end

# Wheter the cluster is allowed or not based on the configured
# clusters and if the cluster allows jobs (job_allow?)
#
# @return [Boolean] whether the cluster is allowed or not
def cluster_allowed(cluster)
cluster.job_allow? && configured_clusters.any? do |pattern|
File.fnmatch(pattern, cluster.id.to_s, File::FNM_EXTGLOB)
end
end

# The clusters that the batch connect app can use. It's a combination
# of what the app is configured to use and what the user is allowed
# to use.
# @return [OodCore::Clusters] clusters available to the app user
def clusters
cluster_regex = configured_clusters.join('|')

OodAppkit.clusters.select do |cluster|
cluster.id.to_s.match(/^#{cluster_regex}$/) && cluster.job_allow?
end
OodAppkit.clusters.select { |cluster| cluster_allowed(cluster) }
end

# Whether this is a valid app the user can use
Expand Down
51 changes: 29 additions & 22 deletions apps/dashboard/test/models/batch_connect/app_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -127,8 +127,7 @@ def bad_clusters
Dir.mktmpdir { |dir|
r = PathRouter.new(dir)
# note the format here, it's a string not array for backward compatability
# and it's the special case * (not the regex .*). Also note the quotes, those
# are nessecary for yaml to parse it correctly
# Also note the quotes, those are nessecary for yaml to parse it correctly
r.path.join("form.yml").write("cluster: '*'")

app = BatchConnect::App.new(router: r)
Expand All @@ -137,40 +136,26 @@ def bad_clusters
}
end

test "app with all clusters regex" do
test "app with single glob to get owens" do
OodAppkit.stubs(:clusters).returns(good_clusters + bad_clusters)

Dir.mktmpdir { |dir|
r = PathRouter.new(dir)
# not the special case * but the regex .*
r.path.join("form.yml").write("cluster: .*")

app = BatchConnect::App.new(router: r)
assert app.valid?
assert_equal good_clusters, app.clusters # make sure you only allow good clusters
}
end

test "app with single regex to get owens" do
OodAppkit.stubs(:clusters).returns(good_clusters + bad_clusters)

Dir.mktmpdir { |dir|
r = PathRouter.new(dir)
r.path.join("form.yml").write("cluster: o.*")
r.path.join("form.yml").write("cluster: o*")

app = BatchConnect::App.new(router: r)
assert app.valid?
assert_equal [ OodCore::Cluster.new({id: 'owens', job: {foo: 'bar'}}) ], app.clusters
}
end

test "app with single regex to get owens and pitzer" do
test "app with multiple globs to get owens and pitzer, but not ruby" do
OodAppkit.stubs(:clusters).returns(good_clusters + bad_clusters)

Dir.mktmpdir { |dir|
r = PathRouter.new(dir)
# try to pick up owens pitzter and ruby by regexs
r.path.join("form.yml").write("cluster:\n - o.*\n - p.*\n - r.*")
# try to pick up owens pitzer by globs
r.path.join("form.yml").write("cluster:\n - o*\n - p*\n - r*")

app = BatchConnect::App.new(router: r)
assert app.valid?
Expand Down Expand Up @@ -204,7 +189,7 @@ def bad_clusters
}
end

test "app disregards empty cluster strings" do
test "app disregards empty cluster strings" do
OodAppkit.stubs(:clusters).returns(good_clusters + bad_clusters)

Dir.mktmpdir { |dir|
Expand All @@ -221,5 +206,27 @@ def bad_clusters
}
end

test "app does not include quick_pitzer when given pitzer" do
clusters = good_clusters +
[
OodCore::Cluster.new({ id: 'quick_pitzer', job: { foo: 'bar' } }),
OodCore::Cluster.new({ id: 'owens_login', job: { foo: 'bar' } }),
OodCore::Cluster.new({ id: '_owens_', job: { foo: 'bar' } }),
OodCore::Cluster.new({ id: '_pitzer_', job: { foo: 'bar' } }),
OodCore::Cluster.new({ id: 'pit', job: { foo: 'bar' } }),
OodCore::Cluster.new({ id: 'owen', job: { foo: 'bar' } })
]

OodAppkit.stubs(:clusters).returns(clusters + bad_clusters)

Dir.mktmpdir { |dir|
r = PathRouter.new(dir)
r.path.join("form.yml").write("cluster:\n - \"owens\"\n - \"pitzer\"")

app = BatchConnect::App.new(router: r)

assert app.valid?
assert_equal good_clusters, app.clusters
}
end
end