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
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
CreditAttribution: phenaproxima at Acquia commented
Status: Active » Postponed
FileSize
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
CreditAttribution: phenaproxima at Acquia 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
CreditAttribution: phenaproxima at Acquia commented
Status: Needs work » Needs review
FileSize
2503049-4.patch6.98 KB

Fixed broken test.

quietone’s picture

quietone CreditAttribution: quietone commented
FileSize
interdiff-2503049-4-5.txt506 bytes
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 CreditAttribution: 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
CreditAttribution: phenaproxima at Acquia 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
CreditAttribution: phenaproxima at Acquia commented
FileSize
2503049-12.patch6.62 KB
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
CreditAttribution: phenaproxima at Acquia commented
Status: Needs work » Needs review
FileSize
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
CreditAttribution: phenaproxima at Acquia commented
FileSize
2503049-17.patch6.54 KB
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
CreditAttribution: phenaproxima at Acquia commented
FileSize
2503049-18.patch6.66 KB
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
CreditAttribution: phenaproxima at Acquia commented
FileSize
2503049-19.patch6.41 KB
interdiff-2503049-17-19.txt524 bytes

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

chx’s picture

chx CreditAttribution: 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 🇨🇦
CreditAttribution: webchick at Acquia 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
Primary language English
Location Vancouver
CreditAttribution: joelpittet 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 🇨🇦
CreditAttribution: webchick at Acquia 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
CreditAttribution: neclimdul 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
CreditAttribution: phenaproxima at Acquia 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
CreditAttribution: phenaproxima at Acquia 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
CreditAttribution: phenaproxima at Acquia commented
FileSize
interdiff-2503049-19-31.txt2 KB
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 Murphysboro, IL, USA
CreditAttribution: mikeryan at Acquia commented
Status: Needs review » Reviewed & tested by the community
webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick at Acquia 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.