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.

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

Fix ASCA Port Overwrite Issue in .checkmarx.yml (AST-75444) #946

Merged
merged 18 commits into from
Nov 24, 2024
4 changes: 2 additions & 2 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -99,11 +99,11 @@ jobs:
name: ${{ runner.os }}-coverage-latest
path: coverage.html

- name: Check if total coverage is greater then 79.9
- name: Check if total coverage is greater then 79
shell: bash
run: |
CODE_COV=$(go tool cover -func cover.out | grep total | awk '{print substr($3, 1, length($3)-1)}')
EXPECTED_CODE_COV=79.7
EXPECTED_CODE_COV=79
var=$(awk 'BEGIN{ print "'$CODE_COV'"<"'$EXPECTED_CODE_COV'" }')
if [ "$var" -eq 1 ];then
echo "Your code coverage is too low. Coverage precentage is: $CODE_COV"
Expand Down
1 change: 1 addition & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ linters-settings:
- github.com/MakeNowJust/heredoc
- github.com/jsumners/go-getport
- github.com/stretchr/testify/assert
- github.com/gofrs/flock
dupl:
threshold: 500
funlen:
Expand Down
3 changes: 2 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ require (
github.com/CheckmarxDev/containers-resolver v1.0.14
github.com/MakeNowJust/heredoc v1.0.0
github.com/bouk/monkey v1.0.0
github.com/gofrs/flock v0.8.1
github.com/golang-jwt/jwt v3.2.2+incompatible
github.com/gomarkdown/markdown v0.0.0-20241102151059-6bc1ffdc6e8c
github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510
Expand All @@ -26,6 +27,7 @@ require (
golang.org/x/text v0.17.0
google.golang.org/grpc v1.65.0
google.golang.org/protobuf v1.34.2
gopkg.in/yaml.v3 v3.0.1
gotest.tools v2.2.0+incompatible
)

Expand Down Expand Up @@ -261,7 +263,6 @@ require (
gopkg.in/ini.v1 v1.67.0 // indirect
gopkg.in/warnings.v0 v0.1.2 // indirect
gopkg.in/yaml.v2 v2.4.0 // indirect
gopkg.in/yaml.v3 v3.0.1 // indirect
helm.sh/helm/v3 v3.15.4 // indirect
k8s.io/api v0.30.3 // indirect
k8s.io/apiextensions-apiserver v0.30.3 // indirect
Expand Down
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -388,6 +388,8 @@ github.com/gobuffalo/packr/v2 v2.8.3/go.mod h1:0SahksCVcx4IMnigTjiFuyldmTrdTctXs
github.com/gobwas/glob v0.2.3 h1:A4xDbljILXROh+kObIiy5kIaPYD8e96x1tgBhUI5J+Y=
github.com/gobwas/glob v0.2.3/go.mod h1:d3Ez4x06l9bZtSvzIay5+Yzi0fmZzPgnTbPcKjJAkT8=
github.com/godbus/dbus/v5 v5.0.4/go.mod h1:xhWf0FNVPg57R7Z0UbKHbJfkEywrmjJnf7w5xrFpKfA=
github.com/gofrs/flock v0.8.1 h1:+gYjHKf32LDeiEEFhQaotPbLuUXjY5ZqxKgXy7n59aw=
github.com/gofrs/flock v0.8.1/go.mod h1:F1TvTiK9OcQqauNUHlbJvyl9Qa1QvF/gOUDKA14jxHU=
github.com/gogo/protobuf v1.1.1/go.mod h1:r8qH/GZQm5c6nD/R0oafs1akxWv10x8SbQlK7atdtwQ=
github.com/gogo/protobuf v1.3.2 h1:Ov1cvc58UF3b5XjBnZv7+opcTcQFZebYjWzi34vdm4Q=
github.com/gogo/protobuf v1.3.2/go.mod h1:P1XiOD3dCwIKUDQYPy72D8LYyHL2YPYrpS2s69NZV8Q=
Expand Down
71 changes: 71 additions & 0 deletions internal/commands/util/configuration_test.go
Original file line number Diff line number Diff line change
@@ -1,11 +1,17 @@
package util

import (
"os"
"strings"
"testing"

"github.com/checkmarx/ast-cli/internal/wrappers/configuration"
asserts "github.com/stretchr/testify/assert"
"gotest.tools/assert"
)

const cxAscaPort = "cx_asca_port"

func TestNewConfigCommand(t *testing.T) {
cmd := NewConfigCommand()
assert.Assert(t, cmd != nil, "Config command must exist")
Expand All @@ -23,3 +29,68 @@ func TestNewConfigCommand(t *testing.T) {
assert.Assert(t, err != nil)
assert.Assert(t, err.Error() == "Failed to set property: unknown property or bad value")
}

func TestGetConfigFilePath_CheckmarxConfigFileExists_Success(t *testing.T) {
want := ".checkmarx/checkmarxcli.yaml"
got, err := configuration.GetConfigFilePath()

if err != nil {
t.Errorf("GetConfigFilePath() error = %v, wantErr = false", err)
return
}

asserts.True(t, strings.HasSuffix(got, want), "Expected config file path to end with %q, but got %q", want, got)
}

func TestWriteSingleConfigKeyToExistingFile_ChangeAscaPortToZero_Success(t *testing.T) {
configuration.LoadConfiguration()
configFilePath, _ := configuration.GetConfigFilePath()
err := configuration.SafeWriteSingleConfigKey(configFilePath, cxAscaPort, 0)
assert.NilError(t, err)
AlvoBen marked this conversation as resolved.
Show resolved Hide resolved

config, err := configuration.LoadConfig(configFilePath)
assert.NilError(t, err)
asserts.Equal(t, 0, config[cxAscaPort])
}

func TestWriteSingleConfigKeyNonExistingFile_CreatingTheFileAndWritesTheKey_Success(t *testing.T) {
configFilePath := "non-existing-file"

file, err := os.Open(configFilePath)
asserts.NotNil(t, err)
asserts.Nil(t, file)

err = configuration.SafeWriteSingleConfigKey(configFilePath, cxAscaPort, 0)
assert.NilError(t, err)

file, err = os.Open(configFilePath)
assert.NilError(t, err)
defer func(file *os.File) {
_ = file.Close()
_ = os.Remove(configFilePath)
_ = os.Remove(configFilePath + ".lock")
}(file)
asserts.NotNil(t, file)
}

func TestChangedOnlyAscaPortInConfigFile_ConfigFileExistsWithDefaultValues_OnlyAscaPortChangedSuccess(t *testing.T) {
configuration.LoadConfiguration()
configFilePath, _ := configuration.GetConfigFilePath()

oldConfig, err := configuration.LoadConfig(configFilePath)
assert.NilError(t, err)

err = configuration.SafeWriteSingleConfigKey(configFilePath, cxAscaPort, -1)
assert.NilError(t, err)

config, err := configuration.LoadConfig(configFilePath)
assert.NilError(t, err)
asserts.Equal(t, -1, config[cxAscaPort])

// Assert all the other properties are the same
for key, value := range oldConfig {
if key != cxAscaPort {
asserts.Equal(t, value, config[key])
}
}
}
18 changes: 10 additions & 8 deletions internal/services/asca.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"github.com/checkmarx/ast-cli/internal/params"
"github.com/checkmarx/ast-cli/internal/services/osinstaller"
"github.com/checkmarx/ast-cli/internal/wrappers"
"github.com/checkmarx/ast-cli/internal/wrappers/configuration"
"github.com/checkmarx/ast-cli/internal/wrappers/grpcs"
getport "github.com/jsumners/go-getport"
"github.com/spf13/viper"
Expand Down Expand Up @@ -110,7 +111,15 @@ func findASCAPort() (int, error) {
if err != nil {
return 0, err
}
setConfigPropertyQuiet(params.ASCAPortKey, port)
viper.Set(params.ASCAPortKey, port)
configFilePath, err := configuration.GetConfigFilePath()
if err != nil {
logger.PrintfIfVerbose("Error getting config file path: %v", err)
}
err = configuration.SafeWriteSingleConfigKey(configFilePath, params.ASCAPortKey, port)
if err != nil {
logger.PrintfIfVerbose("Error writing ASCA port to config file: %v", err)
}
return port, nil
}

Expand All @@ -133,13 +142,6 @@ func configureASCAWrapper(existingASCAWrapper grpcs.AscaWrapper) (grpcs.AscaWrap
return existingASCAWrapper, nil
}

func setConfigPropertyQuiet(propName string, propValue int) {
viper.Set(propName, propValue)
if viperErr := viper.SafeWriteConfig(); viperErr != nil {
_ = viper.WriteConfig()
}
}

func ensureASCAServiceRunning(wrappersParam AscaWrappersParam, ascaParams AscaScanParams) error {
if err := wrappersParam.ASCAWrapper.HealthCheck(); err != nil {
err = checkLicense(ascaParams.IsDefaultAgent, wrappersParam)
Expand Down
80 changes: 80 additions & 0 deletions internal/wrappers/configuration/configuration.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,10 @@ import (
"strings"

"github.com/checkmarx/ast-cli/internal/params"
"github.com/gofrs/flock"
"github.com/pkg/errors"
"github.com/spf13/viper"
"gopkg.in/yaml.v3"
)

const configDirName = "/.checkmarx"
Expand Down Expand Up @@ -128,6 +131,83 @@ func LoadConfiguration() {
_ = viper.ReadInConfig()
}

func SafeWriteSingleConfigKey(configFilePath, key string, value int) error {
// Create a file lock
lock := flock.New(configFilePath + ".lock")
AlvoBen marked this conversation as resolved.
Show resolved Hide resolved
locked, err := lock.TryLock()
if err != nil {
return errors.Errorf("error acquiring lock: %s", err.Error())
}
if !locked {
return errors.Errorf("could not acquire lock")
}
defer func() {
_ = lock.Unlock()
}()

// Load existing configuration or initialize a new one
config, err := LoadConfig(configFilePath)
if err != nil {
return errors.Errorf("error loading config: %s", err.Error())
}

// Update the configuration key
config[key] = value

// Save the updated configuration back to the file
if err = SaveConfig(configFilePath, config); err != nil {
return errors.Errorf("error saving config: %s", err.Error())
}
return nil
}

// LoadConfig loads the configuration from a file. If the file does not exist, it returns an empty map.
func LoadConfig(path string) (map[string]interface{}, error) {
config := make(map[string]interface{})
file, err := os.Open(path)
if err != nil {
if os.IsNotExist(err) {
return config, nil // Return an empty config if the file doesn't exist
}
return nil, err
}
defer func(file *os.File) {
_ = file.Close()
}(file)

decoder := yaml.NewDecoder(file)
if err = decoder.Decode(&config); err != nil {
OrShamirCM marked this conversation as resolved.
Show resolved Hide resolved
return nil, fmt.Errorf("error decoding YAML: %w", err)
}
return config, nil
}

// SaveConfig writes the configuration to a file.
func SaveConfig(path string, config map[string]interface{}) error {
file, err := os.Create(path)
if err != nil {
return err
}

defer func(file *os.File) {
_ = file.Close()
}(file)

encoder := yaml.NewEncoder(file)
if err = encoder.Encode(config); err != nil {
return fmt.Errorf("error encoding YAML: %w", err)
}
return nil
}

func GetConfigFilePath() (string, error) {
usr, err := user.Current()
if err != nil {
return "", fmt.Errorf("error getting current user: %w", err)
}
return usr.HomeDir + configDirName + "/checkmarxcli.yaml", nil
}

func verifyConfigDir(fullPath string) {
if _, err := os.Stat(fullPath); os.IsNotExist(err) {
fmt.Println("Creating directory")
Expand Down
Loading