Skip to content

Commit

Permalink
Merge pull request #617 from OSC/bc-regex-bug
Browse files Browse the repository at this point in the history
Bc regex bug
  • Loading branch information
ericfranz authored Jul 31, 2020
2 parents e1904e7 + aecec5d commit 58d8bc7
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 28 deletions.
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

0 comments on commit 58d8bc7

Please sign in to comment.