Skip to content

Commit

Permalink
Addressed review comments and refactored code accordingly.
Browse files Browse the repository at this point in the history
  • Loading branch information
@renormalize
renormalize committed Nov 14, 2023
1 parent 9c4af90 commit fd46a0c
Show file tree
Hide file tree
Showing 8 changed files with 61 additions and 72 deletions.
28 changes: 10 additions & 18 deletions pkg/snapshot/snapshotter/snapshotter.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -736,34 +736,26 @@ func (ssr *Snapshotter) resetFullSnapshotTimer() error {
return nil
}

// checkSnapstoreSecretUpdate optimized by checking file modfication timestamp instead of computing the hash
// checkSnapstoreSecretUpdate checks if the snapstore secret has been updated
func (ssr *Snapshotter) checkSnapstoreSecretUpdate() bool {
ssr.logger.Debug("checking the timestamp of snapstore secret...")
newSnapstoreSecretModTime, err := snapstore.GetSnapstoreSecretModTime(ssr.snapstoreConfig)
if err != nil {
ssr.logger.Info("Error ", err.Error())
return true
}
// TODO: @renormalize remove the two debug statements
ssr.logger.Info("Debug: Old Mod Time: ", snapstorePrevModTime)
ssr.logger.Info("Debug: New Mod Time: ", newSnapstoreSecretModTime)
newSecretModTime, err := snapstore.GetSnapstoreSecretModTime(ssr.snapstoreConfig)

// newSnapstoreSecretModTime should not be zero, update snapstore
if newSnapstoreSecretModTime.IsZero() {
// returning true to force recreation of the snapstore object
// creation ensures all dependent entities are in the expected format (thus reusing error handling)
// errors that exist will be detected while creation and handled appropriately with verbose logging
if err != nil {
ssr.logger.Info("Error checking the access credential modification time: ", err.Error())
return true
}

// the secret has not been modified
if newSnapstoreSecretModTime.Equal(snapstorePrevModTime) {
ssr.logger.Info("Access credentials not updated...")
if !newSecretModTime.After(snapstorePrevModTime) {
return false
}

// access credentials in the secret were modified
// caveat -> the first delta updates the mod time from time.Time{} to a non zero value
// thus the snapstore object is always updated in the first delta snapshot
snapstorePrevModTime = newSnapstoreSecretModTime
ssr.logger.Info("Access credentials updated...")
// update the previous modification time with the latest modification time
snapstorePrevModTime = newSecretModTime
return true
}

Expand Down
16 changes: 8 additions & 8 deletions pkg/snapstore/abs_snapstore.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"net/url"
"os"
"path"
"path/filepath"
"sort"
"strings"
"sync"
Expand Down Expand Up @@ -353,24 +354,24 @@ func ABSSnapStoreModTime(config *brtypes.SnapstoreConfig) (time.Time, error) {
if dir, isSet := os.LookupEnv(absCredentialFile); isSet {
absTimeStamp, err := readABSCredentialModTime(dir)
if err != nil {
return time.Time{}, fmt.Errorf("error getting credentials using %v directory with error: %v", dir, err.Error())
return time.Time{}, fmt.Errorf("failed to get ABS credentials from the directory %v with error: %v", dir, err.Error())
}
return absTimeStamp, nil
}

if filename, isSet := os.LookupEnv(absCredentialJSONFile); isSet {
fileInfo, err := os.Stat(filename)
if err != nil {
return time.Time{}, fmt.Errorf("error getting json credentials %v with error %v", filename, err.Error())
return time.Time{}, fmt.Errorf("failed to fetch file information of the ABS JSON credential file %v with error: %v", filename, err.Error())
}
absTimeStamp, err := readModTime(fileInfo)
if err != nil {
return time.Time{}, fmt.Errorf("error getting credentials using %v file with error: %v", filename, err.Error())
return time.Time{}, fmt.Errorf("failed to read file information of the ABS JSON credential file %v with error: %v", filename, err.Error())
}
return absTimeStamp, nil
}

return time.Time{}, fmt.Errorf("error, no env variable with the file")
return time.Time{}, fmt.Errorf("No environment variable set for the ABS credential file")
}

// readABSCredentialModTime returns the latest modification
Expand All @@ -385,19 +386,18 @@ func readABSCredentialModTime(dirname string) (time.Time, error) {

for _, file := range files {
// os.Stat instead of file.Info because the file is symlinked
fileInfo, err := os.Stat(dirname + "/" + file.Name())
fileInfo, err := os.Stat(filepath.Join(dirname, file.Name()))
if err != nil {
return time.Time{}, err
}
// storageKey and storageAccount were used to calculate the hash
switch fileInfo.Name() {
case "storageKey", "storageAccount":
fileModTime, err := readModTime(fileInfo)
if err != nil {
return time.Time{}, fmt.Errorf("abs credentials: storageKey or storageAccount is missing")
return time.Time{}, fmt.Errorf("error reading modification time for ABS credential file %v with error: %v", fileInfo.Name(), err.Error())
}
// if the file was modified after the currently stored timestamp, update
if latestModTime.Compare(fileModTime) == -1 {
if latestModTime.Before(fileModTime) {
latestModTime = fileModTime
}
}
Expand Down
6 changes: 3 additions & 3 deletions pkg/snapstore/gcs_snapstore.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -265,14 +265,14 @@ func GCSSnapStoreModTime(config *brtypes.SnapstoreConfig) (time.Time, error) {
if filename, isSet := os.LookupEnv(storeCredentials); isSet {
fileInfo, err := os.Stat(filename)
if err != nil {
return time.Time{}, fmt.Errorf("error getting json credentials %v with error %v", filename, err.Error())
return time.Time{}, fmt.Errorf("failed to fetch file information of the GCS JSON credential file %v with error: %v", filename, err.Error())
}
gcsTimeStamp, err := readModTime(fileInfo)
if err != nil {
return time.Time{}, fmt.Errorf("error getting credentials using %v file with error: %v", filename, err.Error())
return time.Time{}, fmt.Errorf("failed to read file information of the GCS JSON credential file %v with error: %v", filename, err.Error())
}
return gcsTimeStamp, nil
}

return time.Time{}, fmt.Errorf("error, no env variable with the file")
return time.Time{}, fmt.Errorf("No environment variable set for the GCS credential file")
}
16 changes: 8 additions & 8 deletions pkg/snapstore/ocs_s3_snapstore.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"encoding/json"
"fmt"
"os"
"path/filepath"
"strconv"
"time"

Expand Down Expand Up @@ -183,24 +184,24 @@ func OCSSnapStoreModTime(config *brtypes.SnapstoreConfig) (time.Time, error) {
if dir, isSet := os.LookupEnv(ocsCredentialFile); isSet {
ocsTimeStamp, err := readOCSCredentialModTime(dir)
if err != nil {
return time.Time{}, fmt.Errorf("error getting credentials using %v directory with error: %v", dir, err.Error())
return time.Time{}, fmt.Errorf("failed to get OCS credentials from the directory %v with error: %v", dir, err.Error())
}
return ocsTimeStamp, nil
}

if filename, isSet := os.LookupEnv(ocsCredentialJSONFile); isSet {
fileInfo, err := os.Stat(filename)
if err != nil {
return time.Time{}, fmt.Errorf("error getting json credentials %v with error %v", filename, err.Error())
return time.Time{}, fmt.Errorf("failed to fetch file information of the OCS JSON credential file %v with error: %v", filename, err.Error())
}
ocsTimeStamp, err := readModTime(fileInfo)
if err != nil {
return time.Time{}, fmt.Errorf("error getting credentials using %v file with error: %v", filename, err.Error())
return time.Time{}, fmt.Errorf("failed to read file information of the OCS JSON credential file %v with error: %v", filename, err.Error())
}
return ocsTimeStamp, nil
}

return time.Time{}, fmt.Errorf("error, no env variable with the file")
return time.Time{}, fmt.Errorf("No environment variable set for the OCS credential file")
}

// readOCSCredentialModTime returns the latest modification
Expand All @@ -215,19 +216,18 @@ func readOCSCredentialModTime(dirname string) (time.Time, error) {

for _, file := range files {
// os.Stat instead of file.Info because the file is symlinked
fileInfo, err := os.Stat(dirname + "/" + file.Name())
fileInfo, err := os.Stat(filepath.Join(dirname, file.Name()))
if err != nil {
return time.Time{}, err
}
// accessKeyID, region, endpoint and secretAccessKey were used to calculate the hash
switch fileInfo.Name() {
case "accessKeyID", "region", "endpoint", "secretAccessKey":
fileModTime, err := readModTime(fileInfo)
if err != nil {
return time.Time{}, fmt.Errorf("ocs s3 credentials: region, secretAccessKey, endpoint or accessKeyID is missing")
return time.Time{}, fmt.Errorf("error reading modification time for OCS credential file %v with error: %v", fileInfo.Name(), err.Error())
}
// if the file was modified after the currently stored timestamp, update
if latestModTime.Compare(fileModTime) == -1 {
if latestModTime.Before(fileModTime) {
latestModTime = fileModTime
}
}
Expand Down
16 changes: 8 additions & 8 deletions pkg/snapstore/oss_snapstore.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"math"
"os"
"path"
"path/filepath"
"sort"
"strings"
"sync"
Expand Down Expand Up @@ -347,24 +348,24 @@ func OSSSnapStoreModTime(config *brtypes.SnapstoreConfig) (time.Time, error) {
if dir, isSet := os.LookupEnv(aliCredentialFile); isSet {
aliTimeStamp, err := readALICredentialModTime(dir)
if err != nil {
return time.Time{}, fmt.Errorf("error getting credentials using %v directory with error: %v", dir, err.Error())
return time.Time{}, fmt.Errorf("failed to get OSS credentials from the directory %v with error: %v", dir, err.Error())
}
return aliTimeStamp, nil
}

if filename, isSet := os.LookupEnv(aliCredentialJSONFile); isSet {
fileInfo, err := os.Stat(filename)
if err != nil {
return time.Time{}, fmt.Errorf("error getting json credentials %v with error %v", filename, err.Error())
return time.Time{}, fmt.Errorf("failed to fetch file information of the OSS JSON credential file %v with error: %v", filename, err.Error())
}
aliTimeStamp, err := readModTime(fileInfo)
if err != nil {
return time.Time{}, fmt.Errorf("error getting credentials using %v file with error: %v", filename, err.Error())
return time.Time{}, fmt.Errorf("failed to read file information of the OSS JSON credential file %v with error: %v", filename, err.Error())
}
return aliTimeStamp, nil
}

return time.Time{}, fmt.Errorf("error, no env variable with the file")
return time.Time{}, fmt.Errorf("No environment variable set for the OSS credential file")
}

// readALICredentialModTime returns the latest modification
Expand All @@ -379,19 +380,18 @@ func readALICredentialModTime(dirname string) (time.Time, error) {

for _, file := range files {
// os.Stat instead of file.Info because the file is symlinked
fileInfo, err := os.Stat(dirname + "/" + file.Name())
fileInfo, err := os.Stat(filepath.Join(dirname, file.Name()))
if err != nil {
return time.Time{}, err
}
// accessKeyID, accessKeySecret and storageEndpoint were used to calculate the hash
switch fileInfo.Name() {
case "accessKeyID", "accessKeySecret", "storageEndpoint":
fileModTime, err := readModTime(fileInfo)
if err != nil {
return time.Time{}, fmt.Errorf("ali oss credentials: storageEndpoint, accessKeySecret or accessKeyID is missing")
return time.Time{}, fmt.Errorf("error reading modification time for OSS credential file %v with error: %v", fileInfo.Name(), err.Error())
}
// if the file was modified after the currently stored timestamp, update
if latestModTime.Compare(fileModTime) == -1 {
if latestModTime.Before(fileModTime) {
latestModTime = fileModTime
}
}
Expand Down
16 changes: 8 additions & 8 deletions pkg/snapstore/s3_snapstore.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"net/http"
"os"
"path"
"path/filepath"
"sort"
"strconv"
"strings"
Expand Down Expand Up @@ -500,24 +501,24 @@ func S3SnapStoreModTime(config *brtypes.SnapstoreConfig) (time.Time, error) {
if dir, isSet := os.LookupEnv(awsCredentialFile); isSet {
awsTimeStamp, err := readAWSCredentialModTime(dir)
if err != nil {
return time.Time{}, fmt.Errorf("error getting credentials using %v directory with error: %v", dir, err.Error())
return time.Time{}, fmt.Errorf("failed to get AWS credentials from the directory %v with error: %v", dir, err.Error())
}
return awsTimeStamp, nil
}

if filename, isSet := os.LookupEnv(awsCredentialJSONFile); isSet {
fileInfo, err := os.Stat(filename)
if err != nil {
return time.Time{}, fmt.Errorf("error getting json credentials %v with error %v", filename, err.Error())
return time.Time{}, fmt.Errorf("failed to fetch file information of the AWS JSON credential file %v with error: %v", filename, err.Error())
}
awsTimeStamp, err := readModTime(fileInfo)
if err != nil {
return time.Time{}, fmt.Errorf("error getting credentials using %v file with error: %v", filename, err.Error())
return time.Time{}, fmt.Errorf("failed to read file information of the AWS JSON credential file %v with error: %v", filename, err.Error())
}
return awsTimeStamp, nil
}

return time.Time{}, fmt.Errorf("error, no env variable with the file")
return time.Time{}, fmt.Errorf("No environment variable set for the AWS credential file")
}

// readAWSCredentialModTime returns the latest modification
Expand All @@ -532,19 +533,18 @@ func readAWSCredentialModTime(dirname string) (time.Time, error) {

for _, file := range files {
// os.Stat instead of file.Info because the file is symlinked
fileInfo, err := os.Stat(dirname + "/" + file.Name())
fileInfo, err := os.Stat(filepath.Join(dirname, file.Name()))
if err != nil {
return time.Time{}, err
}
// accessKeyID, region and secretAccessKey were used to calculate the hash
switch fileInfo.Name() {
case "accessKeyID", "region", "secretAccessKey":
fileModTime, err := readModTime(fileInfo)
if err != nil {
return time.Time{}, fmt.Errorf("aws s3 credentials: region, secretAccessKey or accessKeyID is missing")
return time.Time{}, fmt.Errorf("error reading modification time for AWS credential file %v with error: %v", fileInfo.Name(), err.Error())
}
// if the file was modified after the currently stored timestamp, update
if latestModTime.Compare(fileModTime) == -1 {
if latestModTime.Before(fileModTime) {
latestModTime = fileModTime
}
}
Expand Down
19 changes: 8 additions & 11 deletions pkg/snapstore/swift_snapstore.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"math"
"os"
"path"
"path/filepath"
"sort"
"strings"
"sync"
Expand Down Expand Up @@ -496,24 +497,24 @@ func SwiftSnapStoreModTime(config *brtypes.SnapstoreConfig) (time.Time, error) {
if dir, isSet := os.LookupEnv(swiftCredentialFile); isSet {
swiftTimeStamp, err := readSwiftCredentialModTime(dir)
if err != nil {
return time.Time{}, fmt.Errorf("error getting credentials using %v directory with error: %v", dir, err.Error())
return time.Time{}, fmt.Errorf("failed to get Swift credentials from the directory %v with error: %v", dir, err.Error())
}
return swiftTimeStamp, nil
}

if filename, isSet := os.LookupEnv(swiftCredentialJSONFile); isSet {
fileInfo, err := os.Stat(filename)
if err != nil {
return time.Time{}, fmt.Errorf("error getting json credentials %v with error %v", filename, err.Error())
return time.Time{}, fmt.Errorf("failed to fetch file information of the Swift JSON credential file %v with error: %v", filename, err.Error())
}
swiftTimeStamp, err := readModTime(fileInfo)
if err != nil {
return time.Time{}, fmt.Errorf("error getting credentials using %v file with error: %v", filename, err.Error())
return time.Time{}, fmt.Errorf("failed to read file information of the Swift JSON credential file %v with error: %v", filename, err.Error())
}
return swiftTimeStamp, nil
}

return time.Time{}, fmt.Errorf("error, no env variable with the file")
return time.Time{}, fmt.Errorf("No environment variable set for the Swift credential file")
}

// readSwiftCredentialModTime returns the latest modification
Expand All @@ -528,22 +529,18 @@ func readSwiftCredentialModTime(dirname string) (time.Time, error) {

for _, file := range files {
// os.Stat instead of file.Info because the file is symlinked
fileInfo, err := os.Stat(dirname + "/" + file.Name())
fileInfo, err := os.Stat(filepath.Join(dirname, file.Name()))
if err != nil {
return time.Time{}, err
}
// the following files were all used to calculate the hash
// authURL, tenantName, domainName were common to both auth type hash calculation
// authTypeV3ApplicationCredential -> applicationCredentialID, applicationCredentialName, applicationCredentialSecret
// authTypePassword -> username, password
switch fileInfo.Name() {
case "authURL", "tenantName", "domainName", "applicationCredentialID", "applicationCredentialName", "applicationCredentialSecret", "username", "password":
fileModTime, err := readModTime(fileInfo)
if err != nil {
return time.Time{}, fmt.Errorf("swift credentials: region, secretAccessKey or accessKeyID is missing")
return time.Time{}, fmt.Errorf("error reading modification time for Swift credential file %v with error: %v", fileInfo.Name(), err.Error())
}
// if the file was modified after the currently stored timestamp, update
if latestModTime.Compare(fileModTime) == -1 {
if latestModTime.Before(fileModTime) {
latestModTime = fileModTime
}
}
Expand Down
Loading

0 comments on commit fd46a0c

Please sign in to comment.