Skip to content

Commit

Permalink
Address comments part-3
Browse files Browse the repository at this point in the history
  • Loading branch information
@anujc25
anujc25 committed Apr 27, 2023
1 parent cf8de1f commit 205e7dd
Show file tree
Hide file tree
Showing 4 changed files with 55 additions and 28 deletions.
4 changes: 4 additions & 0 deletions pkg/airgapped/plugin_bundle_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,8 @@ imagesToCopy:
tempDir, err := os.MkdirTemp("", "")
Expect(tempDir).ToNot(BeEmpty())
Expect(err).NotTo(HaveOccurred())
defer os.RemoveAll(tempDir)

err = tarinator.UnTarinate(tempDir, dpbo.ToTar)
Expect(err).NotTo(HaveOccurred())

Expand Down Expand Up @@ -301,6 +303,8 @@ imagesToCopy:
tempDir, err := os.MkdirTemp("", "")
Expect(tempDir).ToNot(BeEmpty())
Expect(err).NotTo(HaveOccurred())
defer os.RemoveAll(tempDir)

err = tarinator.UnTarinate(tempDir, dpbo.ToTar)
Expect(err).NotTo(HaveOccurred())

Expand Down
19 changes: 14 additions & 5 deletions pkg/discovery/oci_dbbacked.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,9 @@ func (od *DBBackedOCIDiscovery) fetchInventoryImage() error {
}

// Now that everything is ready, create the digest hash file
_, _ = os.Create(newCacheHashFileForInventoryImage)
if newCacheHashFileForInventoryImage != "" {
_, _ = os.Create(newCacheHashFileForInventoryImage)
}
// Also create digest hash file for inventory metadata image if not empty
if newCacheHashFileForMetadataImage != "" {
_, _ = os.Create(newCacheHashFileForMetadataImage)
Expand All @@ -202,6 +204,8 @@ func (od *DBBackedOCIDiscovery) downloadInventoryDatabase() error {
if err != nil {
return errors.Wrap(err, "unable to create temp directory")
}
defer os.RemoveAll(tempDir1)
defer os.RemoveAll(tempDir2)

// Download the plugin inventory image and save to tempDir1
if err := carvelhelpers.DownloadImageAndSaveFilesToDir(od.image, tempDir1); err != nil {
Expand Down Expand Up @@ -245,7 +249,7 @@ func (od *DBBackedOCIDiscovery) checkImageCache() (string, string) {
log.Fatal(nil, fmt.Sprintf("Fatal: plugins discovery image resolution failed. Please check that the repository image URL %q is correct ", od.image))
}

correctHashFileForInventoryImage := od.checkDigestFileExistance(hashHexValInventoryImage, "")
correctHashFileForInventoryImage := od.checkDigestFileExistence(hashHexValInventoryImage, "")

pluginInventoryMetadataImage, _ := airgapped.GetPluginInventoryMetadataImage(od.image)
_, hashHexValMetadataImage, _ := carvelhelpers.GetImageDigest(pluginInventoryMetadataImage)
Expand All @@ -254,19 +258,24 @@ func (od *DBBackedOCIDiscovery) checkImageCache() (string, string) {
// If image exists a file names `metadata.digest.<hexval>` will be stored
// It is important to store the metadata digest file irrespective of image exists
// or not for future comparisons and validating the cache
correctHashFileForMetadataImage := od.checkDigestFileExistance(hashHexValMetadataImage, "metadata.")
// We do this, for this case:
// - Point the discovery to "image-1" (which has corresponding metadata image defined) [Generally airgapped repository]
// - Later, change to point to discovery "image-2" (which doesn't have corresponding metadata image present) [Generally Production repository]
// The cache invalidation was not happening this time if the digest of "image-1" and "image-2" are same, but since we modify
// the DB content in the air-gapped scenario, we have to invalidate the cache.
correctHashFileForMetadataImage := od.checkDigestFileExistence(hashHexValMetadataImage, "metadata.")

return correctHashFileForInventoryImage, correctHashFileForMetadataImage
}

// checkDigestFileExistance check the digest file already exists in the cache or not
// checkDigestFileExistence check the digest file already exists in the cache or not
// We store the digest hash of the cached DB as a file named "<digestPrefix>digest.<hash>.
// If this file exists, we are done. If not, we remove the current digest file
// as we are about to download a new DB and create a new digest file.
// First check any existing "<digestPrefix>digest.*" file; there should only be one, but
// to protect ourselves, we check first and if there are more then one due
// to some bug, we clean them up and invalidate the cache.
func (od *DBBackedOCIDiscovery) checkDigestFileExistance(hashHexVal, digestPrefix string) string {
func (od *DBBackedOCIDiscovery) checkDigestFileExistence(hashHexVal, digestPrefix string) string {
correctHashFile := filepath.Join(od.pluginDataDir, digestPrefix+"digest."+hashHexVal)
matches, _ := filepath.Glob(filepath.Join(od.pluginDataDir, digestPrefix+"digest.*"))
if len(matches) > 1 {
Expand Down
2 changes: 1 addition & 1 deletion pkg/plugininventory/sqlite_inventory_metadata.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ func (b *SQLiteInventoryMetadata) MergeInventoryMetadataDatabase(additionalMetad
}
defer db.Close()

mergeQuery := `attach ? as additionalMetadataDB;
mergeQuery := `ATTACH ? as additionalMetadataDB;
INSERT OR REPLACE INTO AvailablePluginGroups SELECT Vendor,Publisher,GroupName FROM additionalMetadataDB.AvailablePluginGroups;
INSERT OR REPLACE INTO AvailablePluginBinaries SELECT PluginName,Target,Version FROM additionalMetadataDB.AvailablePluginBinaries;`

Expand Down
58 changes: 36 additions & 22 deletions pkg/plugininventory/sqlite_inventory_metadata_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -129,14 +129,14 @@ var _ = Describe("Unit tests for plugin inventory metadata", func() {
pluginInventory PluginInventory
pluginInventoryFilePath string
additionalMetadataInventoryFilePath string
tmpDir string
tmpDir1, tmpDir2 string
)

createInventoryMetadataDB := func(createSchema bool) (PluginInventoryMetadata, string) {
tmpDir, err = os.MkdirTemp(os.TempDir(), "")
tmpDir1, err = os.MkdirTemp(os.TempDir(), "")
Expect(err).To(BeNil(), "unable to create temporary directory")
// Create empty file for the DB
dbFile, err := os.Create(filepath.Join(tmpDir, SQliteInventoryMetadataDBFileName))
dbFile, err := os.Create(filepath.Join(tmpDir1, SQliteInventoryMetadataDBFileName))
Expect(err).To(BeNil())
mi := NewSQLiteInventoryMetadata(dbFile.Name())
if createSchema {
Expand All @@ -147,14 +147,14 @@ var _ = Describe("Unit tests for plugin inventory metadata", func() {
}

createInventoryDB := func(createSchema bool) (PluginInventory, string) {
tmpDir, err = os.MkdirTemp(os.TempDir(), "")
tmpDir2, err = os.MkdirTemp(os.TempDir(), "")
Expect(err).To(BeNil(), "unable to create temporary directory")

// Create empty file for the DB
dbFile, err := os.Create(filepath.Join(tmpDir, SQliteDBFileName))
dbFile, err := os.Create(filepath.Join(tmpDir2, SQliteDBFileName))
Expect(err).To(BeNil())

inventory := NewSQLiteInventory(dbFile.Name(), tmpDir)
inventory := NewSQLiteInventory(dbFile.Name(), tmpDir2)
if createSchema {
err = inventory.CreateSchema()
Expect(err).To(BeNil(), "failed to create DB table for testing")
Expand All @@ -168,7 +168,8 @@ var _ = Describe("Unit tests for plugin inventory metadata", func() {
metadataInventory, _ = createInventoryMetadataDB(false)
})
AfterEach(func() {
os.RemoveAll(tmpDir)
os.RemoveAll(tmpDir1)
os.RemoveAll(tmpDir2)
})
It("should return an error", func() {
err = metadataInventory.InsertPluginIdentifier(&pluginIdentifier1)
Expand All @@ -183,7 +184,8 @@ var _ = Describe("Unit tests for plugin inventory metadata", func() {
metadataInventory, _ = createInventoryMetadataDB(true)
})
AfterEach(func() {
os.RemoveAll(tmpDir)
os.RemoveAll(tmpDir1)
os.RemoveAll(tmpDir2)
})
It("should not return an error", func() {
err = metadataInventory.InsertPluginIdentifier(&pluginIdentifier1)
Expand All @@ -203,7 +205,8 @@ var _ = Describe("Unit tests for plugin inventory metadata", func() {

})
AfterEach(func() {
os.RemoveAll(tmpDir)
os.RemoveAll(tmpDir1)
os.RemoveAll(tmpDir2)
})
It("should return an error", func() {
err = metadataInventory.InsertPluginIdentifier(&pluginIdentifier1)
Expand All @@ -217,17 +220,18 @@ var _ = Describe("Unit tests for plugin inventory metadata", func() {
Describe("Insert plugin group identifier", func() {
Context("With an empty DB file", func() {
BeforeEach(func() {
tmpDir, err = os.MkdirTemp(os.TempDir(), "")
tmpDir2, err = os.MkdirTemp(os.TempDir(), "")
Expect(err).To(BeNil(), "unable to create temporary directory")

// Create empty file for the DB
dbFile, err := os.Create(filepath.Join(tmpDir, SQliteInventoryMetadataDBFileName))
dbFile, err := os.Create(filepath.Join(tmpDir2, SQliteInventoryMetadataDBFileName))
Expect(err).To(BeNil())

metadataInventory = NewSQLiteInventoryMetadata(dbFile.Name())
})
AfterEach(func() {
os.RemoveAll(tmpDir)
os.RemoveAll(tmpDir1)
os.RemoveAll(tmpDir2)
})
It("should return an error", func() {
err = metadataInventory.InsertPluginGroupIdentifier(&pluginGroupIdentifier1)
Expand All @@ -242,7 +246,8 @@ var _ = Describe("Unit tests for plugin inventory metadata", func() {
metadataInventory, _ = createInventoryMetadataDB(true)
})
AfterEach(func() {
os.RemoveAll(tmpDir)
os.RemoveAll(tmpDir1)
os.RemoveAll(tmpDir2)
})
It("should not return an error", func() {
err = metadataInventory.InsertPluginGroupIdentifier(&pluginGroupIdentifier1)
Expand All @@ -262,7 +267,8 @@ var _ = Describe("Unit tests for plugin inventory metadata", func() {

})
AfterEach(func() {
os.RemoveAll(tmpDir)
os.RemoveAll(tmpDir1)
os.RemoveAll(tmpDir2)
})
It("should return an error", func() {
err = metadataInventory.InsertPluginGroupIdentifier(&pluginGroupIdentifier1)
Expand All @@ -280,7 +286,8 @@ var _ = Describe("Unit tests for plugin inventory metadata", func() {
_, additionalMetadataInventoryFilePath = createInventoryMetadataDB(false)
})
AfterEach(func() {
os.RemoveAll(tmpDir)
os.RemoveAll(tmpDir1)
os.RemoveAll(tmpDir2)
})
It("should return an error", func() {
err = metadataInventory.MergeInventoryMetadataDatabase(additionalMetadataInventoryFilePath)
Expand All @@ -295,7 +302,8 @@ var _ = Describe("Unit tests for plugin inventory metadata", func() {
_, additionalMetadataInventoryFilePath = createInventoryMetadataDB(true)
})
AfterEach(func() {
os.RemoveAll(tmpDir)
os.RemoveAll(tmpDir1)
os.RemoveAll(tmpDir2)
})
It("should not return an error", func() {
err = metadataInventory.MergeInventoryMetadataDatabase(additionalMetadataInventoryFilePath)
Expand All @@ -320,7 +328,8 @@ var _ = Describe("Unit tests for plugin inventory metadata", func() {

})
AfterEach(func() {
os.RemoveAll(tmpDir)
os.RemoveAll(tmpDir1)
os.RemoveAll(tmpDir2)
})
It("should not return an error", func() {
err = metadataInventory.MergeInventoryMetadataDatabase(additionalMetadataInventoryFilePath)
Expand Down Expand Up @@ -348,7 +357,8 @@ var _ = Describe("Unit tests for plugin inventory metadata", func() {
Expect(err).NotTo(HaveOccurred())
})
AfterEach(func() {
os.RemoveAll(tmpDir)
os.RemoveAll(tmpDir1)
os.RemoveAll(tmpDir2)
})
It("should not return an error", func() {
err = metadataInventory.MergeInventoryMetadataDatabase(additionalMetadataInventoryFilePath)
Expand All @@ -364,7 +374,8 @@ var _ = Describe("Unit tests for plugin inventory metadata", func() {
_, pluginInventoryFilePath = createInventoryDB(false)
})
AfterEach(func() {
os.RemoveAll(tmpDir)
os.RemoveAll(tmpDir1)
os.RemoveAll(tmpDir2)
})
It("should return an error", func() {
err = metadataInventory.UpdatePluginInventoryDatabase(pluginInventoryFilePath)
Expand All @@ -379,7 +390,8 @@ var _ = Describe("Unit tests for plugin inventory metadata", func() {
_, pluginInventoryFilePath = createInventoryDB(true)
})
AfterEach(func() {
os.RemoveAll(tmpDir)
os.RemoveAll(tmpDir1)
os.RemoveAll(tmpDir2)
})
It("should return an error", func() {
err = metadataInventory.UpdatePluginInventoryDatabase(pluginInventoryFilePath)
Expand All @@ -394,7 +406,8 @@ var _ = Describe("Unit tests for plugin inventory metadata", func() {
_, pluginInventoryFilePath = createInventoryDB(true)
})
AfterEach(func() {
os.RemoveAll(tmpDir)
os.RemoveAll(tmpDir1)
os.RemoveAll(tmpDir2)
})
It("should not return an error", func() {
err = metadataInventory.UpdatePluginInventoryDatabase(pluginInventoryFilePath)
Expand All @@ -419,7 +432,8 @@ var _ = Describe("Unit tests for plugin inventory metadata", func() {
Expect(err).NotTo(HaveOccurred())
})
AfterEach(func() {
os.RemoveAll(tmpDir)
os.RemoveAll(tmpDir1)
os.RemoveAll(tmpDir2)
})
It("when metadata database tables are empty, it should not return an error and remove all plugin and plugin groups from inventory database", func() {
err = metadataInventory.UpdatePluginInventoryDatabase(pluginInventoryFilePath)
Expand Down

0 comments on commit 205e7dd

Please sign in to comment.