Fix/ub 621 image vulnerabilities

[hfeish]

Remove sqlite3

---

This change is 

[coveralls]

Coverage decreased (-1.9%) to 51.853% when pulling be3276e1526aa93b9f46ff303eb402b655650472 on fix/UB-621_image_vulnerabilities into 0be9ca83ae56d1418378c96567ab5062038664b9 on dev.

[shay-berman]

please remove the un relevant lines instead of putting them in comment, then send me again to review.

---

Review status: 0 of 16 files reviewed at latest revision, all discussions resolved, some commit checks failed.

---

database/connection_test.go, line 19 at r1 (raw file):

package database_test

/*

why you remove this file?

is it sqlite related only? doesn't look like

---

database/init.go, line 110 at r1 (raw file):

}

/*

I don't like you put in comments Just delete the un relevant lines instead of comment out. no reason to leave it there.

---

Comments from Reviewable

[shay-berman]

@fei, let me know when you apply my comments please.

---

Review status: 0 of 16 files reviewed at latest revision, 2 unresolved discussions, some commit checks failed.

---

Comments from Reviewable

[hfeish]

@Shay, I can start to apply this after finish the flex log, if it is not urgent, I will apply it after finish the idempotent issue, how do you think?

---

Review status: 0 of 16 files reviewed at latest revision, 2 unresolved discussions, some commit checks failed.

---

Comments from Reviewable

[shay-berman]

hi please continue with this vulnerability thing after you finish the flex log.

and after vulnerability fix move on to provisioning idempotent (since idempotent will take u more time) ok?

---

Review status: 0 of 16 files reviewed at latest revision, 2 unresolved discussions, some commit checks failed.

---

Comments from Reviewable

[hfeish]

@shay-berman OK

[hfeish]

Review status: 0 of 17 files reviewed at latest revision, 2 unresolved discussions, some commit checks failed.

---

database/connection_test.go, line 19 at r1 (raw file):

Previously, shay-berman wrote…

why you remove this file? is it sqlite related only? doesn't look like

For sqlite, we can test it with a db file which exist But for postgres, we need a running postgres, so I think we cannot test it here, so remove it

---

database/init.go, line 110 at r1 (raw file):

Previously, shay-berman wrote…

I don't like you put in comments Just delete the un relevant lines instead of comment out. no reason to leave it there.

Delete the un relevant lines

---

Comments from Reviewable

[shay-berman]

Review status: 0 of 17 files reviewed at latest revision, 2 unresolved discussions, some commit checks failed.

---

database/connection_test.go, line 19 at r1 (raw file):

Previously, hfeish (Fei Huang) wrote…

For sqlite, we can test it with a db file which exist But for postgres, we need a running postgres, so I think we cannot test it here, so remove it

yes and no

yes currently it tested sqlite but if you remove this unit test it means we have zero unit testing on database/connection.go which is not good.

So please try to add connection_test.go file that will test the open\close of the connection.go you can use mocking ConnectionFactory maybe you should also add NewConnectionWithGlobalConnectionFactory() so you will be able to init the connection with mocked GlobalConnectionFactory. Try to do it if you can so at least we will have unit test on this area as well.

---

local/clients.go, line 31 at r2 (raw file):

  // TODO need to refactor and load all the existing clients automatically (instead of hardcore each one here)
  clients := make(map[string]resources.StorageClient)
  spectrumClient, err := spectrumscale.NewSpectrumLocalClient(logger, config, database)

I am not sure if you can remove this, since it will break scale for sure.

Instead, please put it in comment and send email to Gaurang S. Tapase that in dev branch we moving to postgress only (dropping sqlite) and they should be aware that the scale local client also need to change according (CC me as well).

---

Comments from Reviewable

[shay-berman]

Review status: all files reviewed at latest revision, 3 unresolved discussions, some commit checks failed.

---

Comments from Reviewable

[hfeish]

Review status: 14 of 17 files reviewed at latest revision, 2 unresolved discussions, some commit checks failed.

---

database/connection_test.go, line 19 at r1 (raw file):

Previously, shay-berman wrote…

yes and no yes currently it tested sqlite but if you remove this unit test it means we have zero unit testing on database/connection.go which is not good. So please try to add connection_test.go file that will test the open\close of the connection.go you can use mocking ConnectionFactory maybe you should also add NewConnectionWithGlobalConnectionFactory() so you will be able to init the connection with mocked GlobalConnectionFactory. Try to do it if you can so at least we will have unit test on this area as well.

Add testCorrectFactory here, and in func(f testCorrectFactory) newConnection(), new a gorm.DB, with this we can test open(), and for close(), we test the case while db== nil.

---

local/clients.go, line 31 at r2 (raw file):

Previously, shay-berman wrote…

I am not sure if you can remove this, since it will break scale for sure. Instead, please put it in comment and send email to Gaurang S. Tapase that in dev branch we moving to postgress only (dropping sqlite) and they should be aware that the scale local client also need to change according (CC me as well).

Sent email to Gaurang

---

Comments from Reviewable

[hfeish]

@shay-berman , please give comment here, thanks!

[shay-berman]

---

Review status: 14 of 17 files reviewed at latest revision, 1 unresolved discussion, some commit checks failed.

---

database/connection_test.go, line 19 at r1 (raw file):

Previously, hfeish (Fei Huang) wrote…

Add testCorrectFactory here, and in func(f *testCorrectFactory) newConnection(), new a *gorm.DB, with this we can test open(), and for close(), we test the case while db== nil.

Now its better thanks

---

Comments from Reviewable

[hfeish]

Change back "openssl=1.0.2o-r0" which is latest version