Merge branch 'jswain_whats_new_better_url_validation' into 'master'

Better ReleaseHighlight validation

See merge request gitlab-org/gitlab!55009

lib/release_highlights/validator/entry.rb

@@ -11,7 +11,7 @@ module ReleaseHighlights
 
     validates :title, :body, :stage, presence: true
     validates :'self-managed', :'gitlab-com', inclusion: { in: [true, false], message: "must be a boolean" }
-    validates :url, :image_url, format: { with: URI::DEFAULT_PARSER.make_regexp, message: 'must be a URL' }
+    validates :url, :image_url, public_url: { dns_rebind_protection: true }
     validates :release, numericality: true
     validate :validate_published_at
     validate :validate_packages

scripts/verify-tff-mapping

@@ -110,6 +110,12 @@ tests = [
     explanation: 'Migration should map to its timestamped spec',
     source: 'db/post_migrate/20190924152703_migrate_issue_trackers_data.rb',
     expected: ['spec/migrations/20190924152703_migrate_issue_trackers_data_spec.rb']
+  },
+
+  {
+    explanation: 'Whats New should map to its respective spec',
+    source: 'data/whats_new/202101140001_13_08.yml',
+    expected: ['spec/lib/release_highlights/validator_spec.rb']
   }
 ]
 

spec/lib/release_highlights/validator/entry_spec.rb

@@ -40,8 +40,8 @@ RSpec.describe ReleaseHighlights::Validator::Entry do
       end
 
       it 'validates boolean value of "self-managed" and "gitlab-com"' do
-        allow(entry).to receive(:value_for).with('self-managed').and_return('nope')
-        allow(entry).to receive(:value_for).with('gitlab-com').and_return('yerp')
+        allow(entry).to receive(:value_for).with(:'self-managed').and_return('nope')
+        allow(entry).to receive(:value_for).with(:'gitlab-com').and_return('yerp')
 
         subject.valid?
 
@@ -50,17 +50,18 @@ RSpec.describe ReleaseHighlights::Validator::Entry do
       end
 
       it 'validates URI of "url" and "image_url"' do
-        allow(entry).to receive(:value_for).with('image_url').and_return('imgur/gitlab_feature.gif')
-        allow(entry).to receive(:value_for).with('url').and_return('gitlab/newest_release.html')
+        stub_env('RSPEC_ALLOW_INVALID_URLS', 'false')
+        allow(entry).to receive(:value_for).with(:image_url).and_return('https://foobar.x/images/ci/gitlab-ci-cd-logo_2x.png')
+        allow(entry).to receive(:value_for).with(:url).and_return('')
 
         subject.valid?
 
-        expect(subject.errors[:url]).to include(/must be a URL/)
-        expect(subject.errors[:image_url]).to include(/must be a URL/)
+        expect(subject.errors[:url]).to include(/must be a valid URL/)
+        expect(subject.errors[:image_url]).to include(/is blocked: Host cannot be resolved or invalid/)
       end
 
       it 'validates release is numerical' do
-        allow(entry).to receive(:value_for).with('release').and_return('one')
+        allow(entry).to receive(:value_for).with(:release).and_return('one')
 
         subject.valid?
 
@@ -68,7 +69,7 @@ RSpec.describe ReleaseHighlights::Validator::Entry do
       end
 
       it 'validates published_at is a date' do
-        allow(entry).to receive(:value_for).with('published_at').and_return('christmas day')
+        allow(entry).to receive(:value_for).with(:published_at).and_return('christmas day')
 
         subject.valid?
 
@@ -76,7 +77,7 @@ RSpec.describe ReleaseHighlights::Validator::Entry do
       end
 
       it 'validates packages are included in list' do
-        allow(entry).to receive(:value_for).with('packages').and_return(['ALL'])
+        allow(entry).to receive(:value_for).with(:packages).and_return(['ALL'])
 
         subject.valid?
 

spec/lib/release_highlights/validator_spec.rb

@@ -78,7 +78,10 @@ RSpec.describe ReleaseHighlights::Validator do
   end
 
   describe 'when validating all files' do
-    it 'they should have no errors' do
+    # Permit DNS requests to validate all URLs in the YAML files
+    it 'they should have no errors', :permit_dns do
+      stub_env('RSPEC_ALLOW_INVALID_URLS', 'false')
+
       expect(described_class.validate_all!).to be_truthy, described_class.error_message
     end
   end

tests.yml

@@ -52,3 +52,7 @@ mapping:
   # EE/FOSS factory should map to factories spec
   - source: (ee/)?spec/factories/.+\.rb
     test: spec/factories_spec.rb
+
+  # Whats New should map to its respective spec
+  - source: data/whats_new/\w*.yml
+    test: spec/lib/release_highlights/validator_spec.rb

tooling/danger/helper.rb

@@ -209,6 +209,7 @@ module Tooling
 
         %r{\Adoc/.*(\.(md|png|gif|jpg))\z} => :docs,
         %r{\A(CONTRIBUTING|LICENSE|MAINTENANCE|PHILOSOPHY|PROCESS|README)(\.md)?\z} => :docs,
+        %r{\Adata/whats_new/} => :docs,
 
         %r{\A(ee/)?app/(assets|views)/} => :frontend,
         %r{\A(ee/)?public/} => :frontend,