refacto(remote-server): optimize execution time of export/import #7749

cgagnaire · 2019-08-01T14:06:40Z

Description

Refactoring export/import routine of remote server.
~~This PR also include @lpinsivy insertBulk function of PR #7714 even though it's not used.~~

(Unit tests have to be reworked as classes have been removed).
(Some code can also be remove from Repository classes).

MySQL user 'centreon' needs "FILE" privilege to use "LOAD DATA INFILE" statement :
GRANT FILE on *.* to 'centreon'@'localhost';

Refs: MON-3998

Type of change

Patch fixing an issue (non-breaking change)
New functionality (non-breaking change)
Breaking change (patch or feature) that might cause side effects breaking part of the Software
Updating documentation (missing information, typo...)

Target serie

2.8.x
18.10.x
19.04.x
19.10.x (master)

How this pull request can be tested ?

Use the source files in a platform with one remote server and multiple pollers behind the remote server.

Checklist

Community contributors & Centreon team

I followed the coding style guidelines provided by Centreon
I have commented my code, especially new classes, functions or any legacy code modified. (docblock)
I have commented my code, especially hard-to-understand areas of the PR.
I have made corresponding changes to the documentation.
I have rebased my development branch on the base branch (master, maintenance).

Centreon team only

I have made sure that the unit tests related to the story are successful.
I have made sure that unit tests cover 80% of the code written for the story.
I have made sure that acceptance tests related to the story are successful (local and CI)

sc979 · 2019-08-05T14:19:51Z

src/Centreon/Infrastructure/CentreonLegacyDB/CentreonDBAdapter.php

+        // (`col_name`, `col_name`,...)
+
+        // Construct SQL statement
+        $sql = "LOAD DATA INFILE '$file'";


as you didn't bound the values, you should sanitize them before sending them to the DB.

sc979 · 2019-08-05T15:54:36Z

src/CentreonRemote/Domain/Exporter/ConfigurationExporter.php

+     * Copy directory recusively
+     */
+    private function recursive_copy($src, $dst) {
+        $dir = opendir($src);


please could you sanitize $src and $dst to avoid RCE (using escapeShellArg maybe)

sc979 · 2019-08-05T16:22:23Z

www/class/config-generate-remote/abstract/host.class.php

+        return 0;
+    }
+
+    protected function getHostTemplates(&$host)


a getSomething method need to send back something

sc979 · 2019-08-05T16:23:25Z

www/class/config-generate-remote/abstract/host.class.php

+            }
+            $loop[$host_id] = 1;
+            if ($host_id == $host_tpl_id) {
+                return 1;


Suggested change

return 1;

return true;

sc979 · 2019-08-05T16:23:45Z

www/class/config-generate-remote/abstract/host.class.php

+            $stack = array_merge($hosts_tpl[$host_id]['htpl'], $stack);
+        }
+
+        return 0;


Suggested change

return 0;

return false;

(used in an if condition in config-generate/servicetemplate.class.php +133 and config-generate-remote/servicetemplate.class.php +101)

sc979 · 2019-08-05T16:23:57Z

www/class/config-generate-remote/abstract/host.class.php

+
+    protected function getHostTimezone(&$host)
+    {
+        # not needed


then remove it

sc979 · 2019-08-05T16:26:13Z

www/class/config-generate-remote/abstract/object.class.php

+
+    protected function createFile($dir)
+    {
+        $full_file = $dir . '/' . $this->subdir . '/' . $this->generate_filename;


sanitize $this->generate_filename to avoid RCE exploit

or use escapeshellarg on $full_file variable

sc979 · 2019-08-28T08:44:47Z

I removed "request changes" taken in account and the outdated comments.
The branch have been rebased

sc979 · 2019-08-28T11:32:00Z

@cgagnaire -> About : "MySQL user 'centreon' needs "FILE" privilege to use "LOAD DATA INFILE" statement :
GRANT FILE on . to 'centreon'@'localhost';" -> On which page should this sentence be added to the doc ?

src/CentreonRemote/Infrastructure/Export/ExportParserJson.php

callapa

Always the sames comments:
Le first letter of class name must be capitalized.
Le class name must be equal to his file.
Remove the '.class' in the name of a class file.
Use a boolean value instead of integer

src/CentreonRemote/Domain/Exporter/ConfigurationExporter.php

callapa · 2019-08-28T11:53:53Z

src/CentreonRemote/Domain/Exporter/ConfigurationExporter.php

+
+                // insert data
+                $exportPathFile = $this->getFile($data[filename]);
+                echo date("Y-m-d H:i:s") . " - INFO - Loading '" . $exportPathFile . "'.\n";


Do not printing message directly from the method.
For example you can inject a logger class instance and use it to log message.

callapa · 2019-08-28T11:54:39Z

src/CentreonRemote/Domain/Exporter/ConfigurationExporter.php

+        } catch (\ErrorException $e) {
+            // rollback changes
+            $db->rollBack();
+            echo date("Y-m-d H:i:s") . " - ERROR - Loading failed.\n";


Same comment as above

src/CentreonRemote/Domain/Exporter/ConfigurationExporter.php

callapa · 2019-08-28T11:55:59Z

src/CentreonRemote/Domain/Exporter/ConfigurationExporter.php

+                if (is_dir($src . '/' . $file)) {
+                    $this->recursiveCopy($src . '/' . $file, $dst . '/' . $file);
+                } else {
+                    echo date("Y-m-d H:i:s") . " - INFO - Copying '" . $src . "/" . $file . "'.\n";


Do not printing message directly from the method.
For example you can inject a logger class instance and use it to log message.

www/class/config-generate-remote/serviceCategory.class.php

www/class/config-generate-remote/servicegroup.class.php

www/class/config-generate-remote/servicetemplate.class.php

www/class/config-generate-remote/timeperiod.class.php

www/class/config-generate-remote/trap.class.php

callapa

Always the same comments:

The first letter of class name must be capitalized.
The class name must be equal to his file.
Remove the '.class' in the name of a class file.
Use a boolean value instead of integer

…neration lib

centreon-bot · 2019-09-17T13:18:25Z

www/class/config-generate-remote/Host.php

+    {
+        $severityId = HostCategory::getInstance($this->dependencyInjector)->getHostSeverityByHostId($hostIdArg);
+        if (!is_null($severityId)) {
+            Relations\HostCategoriesRelation::getInstance($this->dependencyInjector)->addRelation($severityId, $hostIdArg);


WARN: Line exceeds 120 characters; contains 123 characters

Refs: #7810, #7749

kduret mentioned this pull request Aug 2, 2019

enh(remote-server): Reduce exported objects and improve process duration #7692

Closed

15 tasks

kduret changed the title ~~Refacto remote export import~~ refacto(remose-server): optimize execution time of export/import Aug 5, 2019

kduret changed the title ~~refacto(remose-server): optimize execution time of export/import~~ refacto(remote-server): optimize execution time of export/import Aug 5, 2019

sc979 suggested changes Aug 5, 2019

View reviewed changes

sc979 force-pushed the refacto-remote-export-import branch 2 times, most recently from e2da8ca to 356c2b1 Compare August 21, 2019 13:31

centreon deleted a comment from cgagnaire Aug 21, 2019

lpinsivy added area/remote-server kind/performance labels Aug 22, 2019

centreon deleted a comment from cgagnaire Aug 28, 2019

sc979 force-pushed the refacto-remote-export-import branch from 8fa444f to 9a02592 Compare August 28, 2019 07:30

centreon deleted a comment from cgagnaire Aug 28, 2019

sc979 requested review from kduret and callapa August 28, 2019 08:43

kduret reviewed Aug 28, 2019

View reviewed changes

src/CentreonRemote/Infrastructure/Export/ExportParserJson.php Show resolved Hide resolved

kduret reviewed Aug 28, 2019

View reviewed changes

src/CentreonRemote/Infrastructure/Export/ExportParserJson.php Show resolved Hide resolved

callapa suggested changes Aug 28, 2019

View reviewed changes

sc979 force-pushed the refacto-remote-export-import branch 2 times, most recently from 26100fc to 0ca1f8f Compare September 3, 2019 09:55

cgagnaire and others added 5 commits September 4, 2019 11:46

refacto(remote-server): refacto configuration import of remote server

84c752d

refacto(remote-server): fix typos

a709b93

refacto(remote-server): merge configuration and media, add call to ge…

7b30bae

…neration lib

refacto(remote-server): fix interface

fb27aa9

add class to export configuration for remote server

55a5cab

kduret added 23 commits September 12, 2019 11:34

fix coding style

e1909b7

fix namespaces

9ac1248

fix require

ecb4b54

fix coding style

41f1db2

fix unit tests

94da397

remove useless function

700a432

fix coding style

7e5d386

fix require

6e15a04

fix creation of tmp directory

742e83b

fix export

f596ff3

Merge branch 'master' into refacto-remote-export-import

b96015e

fix coding style

15e94e1

fix unit tests

2d1e921

fix usage of constants

f6bfbb5

update path to import data

b7291d4

fix export

0aa2030

add documentation to grant file in database

63d7bbe

set exportContact as boolean

50ebe2d

search properly module objects

1026066

fix module generate

6ebb44f

fix load of module classes

61a2994

append to export files

d586baa

fix coding style

3cfc59e

centreon-bot reviewed Sep 17, 2019

View reviewed changes

fix coding style

d88b84e

kduret merged commit 21c80f0 into master Sep 19, 2019

kduret deleted the refacto-remote-export-import branch September 19, 2019 12:58

cgagnaire added a commit that referenced this pull request Sep 24, 2019

fix(remote-server): adapt nagios_server export columns

21526b2

Refs: #7810, #7749

kduret pushed a commit that referenced this pull request Sep 24, 2019

fix(remote-server): adapt nagios_server export columns (#7871)

6403883

Refs: #7810, #7749

UrBnW mentioned this pull request Oct 9, 2019

[Remote Server] No Broker Statistics (debug / investigation) #7635

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refacto(remote-server): optimize execution time of export/import #7749

refacto(remote-server): optimize execution time of export/import #7749

cgagnaire commented Aug 1, 2019 •

edited

Loading

sc979 Aug 5, 2019

sc979 Aug 5, 2019

sc979 Aug 5, 2019

sc979 Aug 5, 2019

sc979 Aug 5, 2019

sc979 Aug 28, 2019

sc979 Aug 5, 2019

sc979 Aug 5, 2019

sc979 Aug 28, 2019

sc979 commented Aug 28, 2019

sc979 commented Aug 28, 2019

callapa left a comment

callapa Aug 28, 2019

callapa Aug 28, 2019

callapa Aug 28, 2019

callapa left a comment •

edited

Loading

centreon-bot Sep 17, 2019

refacto(remote-server): optimize execution time of export/import #7749

refacto(remote-server): optimize execution time of export/import #7749

Conversation

cgagnaire commented Aug 1, 2019 • edited Loading

Description

Type of change

Target serie

How this pull request can be tested ?

Checklist

Community contributors & Centreon team

Centreon team only

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sc979 commented Aug 28, 2019

sc979 commented Aug 28, 2019

callapa left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

callapa left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cgagnaire commented Aug 1, 2019 •

edited

Loading

callapa left a comment •

edited

Loading