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.

Sign up for GitHub

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

Show a setup warning in case S3 object storage is used as primary storage #14603

Merged
merged 1 commit into from
Mar 8, 2019
Merged

Show a setup warning in case S3 object storage is used as primary storage #14603

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
12 changes: 12 additions & 0 deletions core/js/setupchecks.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -450,6 +450,18 @@
type: OC.SetupChecks.MESSAGE_TYPE_WARNING
})
}
if (!data.isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed) {
messages.push({
msg: t(
'core',
'This instance uses an S3 based object store as primary storage. The uploaded files are stored temporarily on the server and thus it is recommended to have 50 GB of free space available in the temp directory of PHP. Check the logs for full details about the path and the available space. To improve this please change the temporary directory in the php.ini or make more space available in that path.',
{
docLink: oc_defaults.docPlaceholderUrl.replace('PLACEHOLDER', 'admin-mysql-utf8mb4'),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@MorrisJobke Is the docLink a copy&paste mistake? I don't see a placeholder in the message and linking to 'admin-mysql-utf8mb4' is obviously wrong in this case.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for noticing - yes this is an c&p mistake.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is the fix: #14620

}
),
type: OC.SetupChecks.MESSAGE_TYPE_WARNING
})
}

} else {
messages.push({
Expand Down
91 changes: 77 additions & 14 deletions core/js/tests/specs/setupchecksSpec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,8 @@ describe('OC.SetupChecks tests', function() {
appDirsWithDifferentOwner: [],
recommendedPHPModules: [],
pendingBigIntConversionColumns: [],
isMysqlUsedWithoutUTF8MB4: false
isMysqlUsedWithoutUTF8MB4: false,
isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: true
})
);

Expand Down Expand Up @@ -299,7 +300,8 @@ describe('OC.SetupChecks tests', function() {
appDirsWithDifferentOwner: [],
recommendedPHPModules: [],
pendingBigIntConversionColumns: [],
isMysqlUsedWithoutUTF8MB4: false
isMysqlUsedWithoutUTF8MB4: false,
isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: true
})
);

Expand Down Expand Up @@ -351,7 +353,8 @@ describe('OC.SetupChecks tests', function() {
appDirsWithDifferentOwner: [],
recommendedPHPModules: [],
pendingBigIntConversionColumns: [],
isMysqlUsedWithoutUTF8MB4: false
isMysqlUsedWithoutUTF8MB4: false,
isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: true
})
);

Expand Down Expand Up @@ -401,7 +404,8 @@ describe('OC.SetupChecks tests', function() {
appDirsWithDifferentOwner: [],
recommendedPHPModules: [],
pendingBigIntConversionColumns: [],
isMysqlUsedWithoutUTF8MB4: false
isMysqlUsedWithoutUTF8MB4: false,
isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: true
})
);

Expand Down Expand Up @@ -449,7 +453,8 @@ describe('OC.SetupChecks tests', function() {
appDirsWithDifferentOwner: [],
recommendedPHPModules: [],
pendingBigIntConversionColumns: [],
isMysqlUsedWithoutUTF8MB4: false
isMysqlUsedWithoutUTF8MB4: false,
isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: true
})
);

Expand Down Expand Up @@ -499,7 +504,8 @@ describe('OC.SetupChecks tests', function() {
],
recommendedPHPModules: [],
pendingBigIntConversionColumns: [],
isMysqlUsedWithoutUTF8MB4: false
isMysqlUsedWithoutUTF8MB4: false,
isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: true
})
);

Expand Down Expand Up @@ -547,7 +553,8 @@ describe('OC.SetupChecks tests', function() {
appDirsWithDifferentOwner: [],
recommendedPHPModules: [],
pendingBigIntConversionColumns: [],
isMysqlUsedWithoutUTF8MB4: false
isMysqlUsedWithoutUTF8MB4: false,
isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: true
})
);

Expand Down Expand Up @@ -595,7 +602,8 @@ describe('OC.SetupChecks tests', function() {
appDirsWithDifferentOwner: [],
recommendedPHPModules: [],
pendingBigIntConversionColumns: [],
isMysqlUsedWithoutUTF8MB4: false
isMysqlUsedWithoutUTF8MB4: false,
isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: true
})
);

Expand Down Expand Up @@ -643,7 +651,8 @@ describe('OC.SetupChecks tests', function() {
appDirsWithDifferentOwner: [],
recommendedPHPModules: [],
pendingBigIntConversionColumns: [],
isMysqlUsedWithoutUTF8MB4: false
isMysqlUsedWithoutUTF8MB4: false,
isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: true
})
);

Expand Down Expand Up @@ -712,7 +721,8 @@ describe('OC.SetupChecks tests', function() {
appDirsWithDifferentOwner: [],
recommendedPHPModules: [],
pendingBigIntConversionColumns: [],
isMysqlUsedWithoutUTF8MB4: false
isMysqlUsedWithoutUTF8MB4: false,
isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: true
})
);

Expand Down Expand Up @@ -761,7 +771,8 @@ describe('OC.SetupChecks tests', function() {
appDirsWithDifferentOwner: [],
recommendedPHPModules: [],
pendingBigIntConversionColumns: [],
isMysqlUsedWithoutUTF8MB4: false
isMysqlUsedWithoutUTF8MB4: false,
isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: true
})
);

Expand Down Expand Up @@ -810,7 +821,8 @@ describe('OC.SetupChecks tests', function() {
appDirsWithDifferentOwner: [],
recommendedPHPModules: [],
pendingBigIntConversionColumns: [],
isMysqlUsedWithoutUTF8MB4: false
isMysqlUsedWithoutUTF8MB4: false,
isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: true
})
);

Expand Down Expand Up @@ -859,7 +871,8 @@ describe('OC.SetupChecks tests', function() {
appDirsWithDifferentOwner: [],
recommendedPHPModules: [],
pendingBigIntConversionColumns: [],
isMysqlUsedWithoutUTF8MB4: false
isMysqlUsedWithoutUTF8MB4: false,
isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: true
})
);

Expand Down Expand Up @@ -907,7 +920,8 @@ describe('OC.SetupChecks tests', function() {
appDirsWithDifferentOwner: [],
recommendedPHPModules: [],
pendingBigIntConversionColumns: [],
isMysqlUsedWithoutUTF8MB4: true
isMysqlUsedWithoutUTF8MB4: true,
isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: true
})
);

Expand All @@ -919,6 +933,55 @@ describe('OC.SetupChecks tests', function() {
done();
});
});

it('should return an error if there is not enough free space in the temp directory', function(done) {
var async = OC.SetupChecks.checkSetup();

suite.server.requests[0].respond(
200,
{
'Content-Type': 'application/json',
},
JSON.stringify({
hasFileinfoInstalled: true,
isGetenvServerWorking: true,
isReadOnlyConfig: false,
hasWorkingFileLocking: true,
hasValidTransactionIsolationLevel: true,
suggestedOverwriteCliURL: '',
isRandomnessSecure: true,
securityDocs: 'https://docs.owncloud.org/myDocs.html',
serverHasInternetConnection: true,
isMemcacheConfigured: true,
forwardedForHeadersWorking: true,
isCorrectMemcachedPHPModuleInstalled: true,
hasPassedCodeIntegrityCheck: true,
isOpcacheProperlySetup: true,
hasOpcacheLoaded: true,
isSettimelimitAvailable: true,
hasFreeTypeSupport: true,
missingIndexes: [],
cronErrors: [],
cronInfo: {
diffInSeconds: 0
},
isMemoryLimitSufficient: true,
appDirsWithDifferentOwner: [],
recommendedPHPModules: [],
pendingBigIntConversionColumns: [],
isMysqlUsedWithoutUTF8MB4: false,
isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed: false
})
);

async.done(function( data, s, x ){
expect(data).toEqual([{
msg: 'This instance uses an S3 based object store as primary storage. The uploaded files are stored temporarily on the server and thus it is recommended to have 50 GB of free space available in the temp directory of PHP. Check the logs for full details about the path and the available space. To improve this please change the temporary directory in the php.ini or make more space available in that path.',
type: OC.SetupChecks.MESSAGE_TYPE_WARNING
}]);
done();
});
});
});

describe('checkGeneric', function() {
Expand Down
37 changes: 37 additions & 0 deletions settings/Controller/CheckSetupController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -628,6 +628,42 @@ protected function hasBigIntConversionPendingColumns(): array {
return $pendingColumns;
}

protected function isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed(): bool {
$objectStore = $this->config->getSystemValue('objectstore', null);
$objectStoreMultibucket = $this->config->getSystemValue('objectstore_multibucket', null);

if (!isset($objectStoreMultibucket) && !isset($objectStore)) {
return true;
}

if (isset($objectStoreMultibucket['class']) && $objectStoreMultibucket['class'] !== 'OC\\Files\\ObjectStore\\S3') {
return true;
}

if (isset($objectStore['class']) && $objectStore['class'] !== 'OC\\Files\\ObjectStore\\S3') {
return true;
}

$tempPath = sys_get_temp_dir();
if (!is_dir($tempPath)) {
$this->logger->error('Error while checking the temporary PHP path - it was not properly set to a directory. value: ' . $tempPath);
return false;
}
$freeSpaceInTemp = disk_free_space($tempPath);
if ($freeSpaceInTemp === false) {
$this->logger->error('Error while checking the available disk space of temporary PHP path - no free disk space returned. temporary path: ' . $tempPath);
return false;
}

$freeSpaceInTempInGB = $freeSpaceInTemp / 1024 / 1024 / 1024;
if ($freeSpaceInTempInGB > 50) {
return true;
}

$this->logger->warning('Checking the available space in the temporary path resulted in ' . round($freeSpaceInTempInGB, 1) . ' GB instead of the recommended 50GB. Path: ' . $tempPath);
return false;
}

/**
* @return DataResponse
*/
Expand Down Expand Up @@ -669,6 +705,7 @@ public function check() {
'recommendedPHPModules' => $this->hasRecommendedPHPModules(),
'pendingBigIntConversionColumns' => $this->hasBigIntConversionPendingColumns(),
'isMysqlUsedWithoutUTF8MB4' => $this->isMysqlUsedWithoutUTF8MB4(),
'isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed' => $this->isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed(),
]
);
}
Expand Down
56 changes: 56 additions & 0 deletions tests/Settings/Controller/CheckSetupControllerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,7 @@ public function setUp() {
'hasRecommendedPHPModules',
'hasBigIntConversionPendingColumns',
'isMysqlUsedWithoutUTF8MB4',
'isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed',
])->getMock();
}

Expand Down Expand Up @@ -526,6 +527,11 @@ public function testCheck() {
->method('isMysqlUsedWithoutUTF8MB4')
->willReturn(false);

$this->checkSetupController
->expects($this->once())
->method('isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed')
->willReturn(true);

$expected = new DataResponse(
[
'isGetenvServerWorking' => true,
Expand Down Expand Up @@ -570,6 +576,7 @@ public function testCheck() {
'recommendedPHPModules' => [],
'pendingBigIntConversionColumns' => [],
'isMysqlUsedWithoutUTF8MB4' => false,
'isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed' => true,
]
);
$this->assertEquals($expected, $this->checkSetupController->check());
Expand Down Expand Up @@ -1400,4 +1407,53 @@ public function testIsMysqlUsedWithoutUTF8MB4(string $db, bool $useUTF8MB4, bool

$this->assertSame($expected, $this->invokePrivate($checkSetupController, 'isMysqlUsedWithoutUTF8MB4'));
}

public function dataForIsEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed() {
return [
['singlebucket', 'OC\\Files\\ObjectStore\\Swift', true],
['multibucket', 'OC\\Files\\ObjectStore\\Swift', true],
['singlebucket', 'OC\\Files\\ObjectStore\\Custom', true],
['multibucket', 'OC\Files\\ObjectStore\\Custom', true],
['singlebucket', 'OC\Files\ObjectStore\Swift', true],
['multibucket', 'OC\Files\ObjectStore\Swift', true],
['singlebucket', 'OC\Files\ObjectStore\Custom', true],
['multibucket', 'OC\Files\ObjectStore\Custom', true],
];
}

/**
* @dataProvider dataForIsEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed
*/
public function testIsEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed(string $mode, string $className, bool $expected) {
$this->config->method('getSystemValue')
->will($this->returnCallback(function($key, $default) use ($mode, $className) {
if ($key === 'objectstore' && $mode === 'singlebucket') {
return ['class' => $className];
}
if ($key === 'objectstore_multibucket' && $mode === 'multibucket') {
return ['class' => $className];
}
return $default;
}));

$checkSetupController = new CheckSetupController(
'settings',
$this->request,
$this->config,
$this->clientService,
$this->urlGenerator,
$this->util,
$this->l10n,
$this->checker,
$this->logger,
$this->dispatcher,
$this->db,
$this->lockingProvider,
$this->dateTimeFormatter,
$this->memoryInfo,
$this->secureRandom
);

$this->assertSame($expected, $this->invokePrivate($checkSetupController, 'isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed'));
}
}