Problem/Motivation

In Drupal 7, IP blocking was handled by the System module, which maintained the blocked_ips table in the database. In Drupal 8, this functionality has been split out into a core module called Ban, and it will need a migration path.

Proposed Resolution

Write a migration to save blocked IPs from Drupal 7 into Drupal 8.

Remaining Tasks

  • Write the patch
  • Write tests
  • Review
  • Merge into the parent issue
CommentFileSizeAuthor
#31 2503049-31.patch6.45 KBphenaproxima
#31 interdiff-2503049-19-31.txt2 KBphenaproxima
#19 interdiff-2503049-17-19.txt524 bytesphenaproxima
#19 2503049-19.patch6.41 KBphenaproxima
#18 interdiff-2503048-17-18.txt5.94 KBphenaproxima
#18 2503049-18.patch6.66 KBphenaproxima
#17 interdiff-2503049-16-17.txt10.91 KBphenaproxima
#17 2503049-17.patch6.54 KBphenaproxima
#16 2503049-16.patch6.59 KBphenaproxima
#12 interdiff-2503049-5-12.txt1.9 KBphenaproxima
#12 2503049-12.patch6.62 KBphenaproxima
#5 2503049-5.patch6.97 KBquietone
#5 interdiff-2503049-4-5.txt506 bytesquietone
#4 2503049-4.patch6.98 KBphenaproxima
#1 2503049-1.patch9.53 KBphenaproxima

Comments

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
Status: Active » Postponed
StatusFileSize
new 2503049-1.patch9.53 KB

Thar be the patch. This is postponed on #2510072: UTF-8 support in MySQL driver breaks migrate dump files, since that issue is holding up the D7 upgrade path.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
Status: Postponed » Needs review

Status: Needs review » Needs work

The last submitted patch, 1: 2503049-1.patch, failed testing.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
Status: Needs work » Needs review
StatusFileSize
new 2503049-4.patch6.98 KB

Fixed broken test.

quietone’s picture

quietone commented
StatusFileSize
new interdiff-2503049-4-5.txt506 bytes
new 2503049-5.patch6.97 KB

Thanks to benjy for getting me started.
I checked that D7 uses a list of IP addresses and not say, a range. Applied patch and tests pass. I did find that the Source provider was 'aggregator', that is changed in the attached patch to 'system.

quietone’s picture

quietone commented
Status: Needs review » Reviewed & tested by the community

Changed to RTBC as suggested on IRC by phenaproxima.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 5: 2503049-5.patch, failed testing.

Status: Needs work » Needs review

phenaproxima queued 5: 2503049-5.patch for re-testing.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
Status: Needs review » Reviewed & tested by the community

Testbot and its mood swings. Back to RTBC.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 5: 2503049-5.patch, failed testing.

Status: Needs work » Needs review

phenaproxima queued 5: 2503049-5.patch for re-testing.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
StatusFileSize
new 2503049-12.patch6.62 KB
new interdiff-2503049-5-12.txt1.9 KB

Just a l'il re-roll...

Status: Needs review » Needs work

The last submitted patch, 12: 2503049-12.patch, failed testing.

phenaproxima queued 12: 2503049-12.patch for re-testing.

The last submitted patch, 12: 2503049-12.patch, failed testing.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
Status: Needs work » Needs review
StatusFileSize
new 2503049-16.patch6.59 KB

Ah! Found the problem. It is a plugin dependency issue -- moving the migration template into the Ban module solves it, as per #2533886: [meta] Move module-specific migration support into the particular modules supported.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
StatusFileSize
new 2503049-17.patch6.54 KB
new interdiff-2503049-16-17.txt10.91 KB

Re-rolled again, moving all the related plugins and tests into the Ban module's space, as per #2533886: [meta] Move module-specific migration support into the particular modules supported.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
StatusFileSize
new 2503049-18.patch6.66 KB
new interdiff-2503048-17-18.txt5.94 KB

As per discussion on IRC with @chx, removed an unneeded comment and migrated the iid column from {ban_ip}.

EDIT: Whoops, overreached there. @chx and I decided not to migrate iid, and instead make it a known issue with the D6-D8 migration path.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
StatusFileSize
new 2503049-19.patch6.41 KB
new interdiff-2503049-17-19.txt524 bytes

Re-rolled from #17, this time just losing the extra comment.

chx’s picture

chx commented
Status: Needs review » Reviewed & tested by the community

We can't migrate iid because there's no API for it. (And ban is broken anyways because the findAll method is returning StatementInterface, boo.) The whole iid business should be dropped preferably before release if it is not too late (but perhaps it is).

mikeryan queued 19: 2503049-19.patch for re-testing.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Reviewed & tested by the community » Fixed

Committed and pushed to 8.0.x. Thanks!

  • webchick committed f9b5e68 on 8.0.x 
    Issue #2503049 by phenaproxima, quietone: Migrate D7 blocked IPs to D8
joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented
Status: Fixed » Needs work

Head seems to be broken on this patch it seems.

https://qa.drupal.org/8.0.x-status
Undefined property: Drupal\ban\Tests\d7\MigrateBlockedIPsTest::$collectMessagesDrupal\migrate\Tests\MigrateTestBase->display('New object was not saved, no error provided') Drupal\migrate\MigrateExecutable->import() Drupal\migrate\Tests\MigrateTestBase->executeMigration('d7_blocked_ips') Drupal\ban\Tests\d7\MigrateBlockedIPsTest->setUp() Drupal\simpletest\TestBase->run(Array) simpletest_script_run_one_test('38', 'Drupal\ban\Tests\d7\MigrateBlockedIPsTest')

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented

Well that's not good! Reverted. Must've conflicted with something else I committed tonight. ;(

  • webchick committed 4b31d8c on 8.0.x 
    Revert "Issue #2503049 by phenaproxima, quietone: Migrate D7 blocked IPs...
neclimdul’s picture

neclimdul
He/Him
commented

#2542318-5: Clean up local variables in MigrateTestBase in tearDown fixes this. sorry guys.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
Status: Needs work » Postponed

Doesn't need a re-roll; just needs the quick-fix #2542318-5: Clean up local variables in MigrateTestBase in tearDown committed. Postponing on that.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
Status: Postponed » Needs review

Let's run this past testbot once more before going back to the Jolly Land of RTBC.

phenaproxima queued 19: 2503049-19.patch for re-testing.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
StatusFileSize
new interdiff-2503049-19-31.txt2 KB
new 2503049-31.patch6.45 KB

Re-rolled anyway, moving the test into its proper namespace (for consistency with the Great Migration Migration).

mikeryan’s picture

mikeryan
he/him
Primary language English
Location Pittsfield, MA, USA
commented
Status: Needs review » Reviewed & tested by the community
webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Reviewed & tested by the community » Fixed

Let's try that again.

Committed and pushed to 8.0.x. Thanks!

  • webchick committed 2b9b8d0 on 8.0.x 
    Issue #2503049 by phenaproxima, quietone, joelpittet: Migrate D7 blocked...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.