diff --git a/OWNERS b/OWNERS index daa8ee3b..0d39974c 100644 --- a/OWNERS +++ b/OWNERS @@ -2,7 +2,6 @@ reviewers: - abyrne55 - reedcort - dakotalongRH - - lnguyen1401 - luis-falcon - joshbranham approvers: diff --git a/boilerplate/_data/backing-image-tag b/boilerplate/_data/backing-image-tag index 7f486aad..10efb639 100644 --- a/boilerplate/_data/backing-image-tag +++ b/boilerplate/_data/backing-image-tag @@ -1 +1 @@ -image-v5.0.0 +image-v6.0.1 diff --git a/boilerplate/_data/last-boilerplate-commit b/boilerplate/_data/last-boilerplate-commit index acbbd7f8..7c9f64be 100644 --- a/boilerplate/_data/last-boilerplate-commit +++ b/boilerplate/_data/last-boilerplate-commit @@ -1 +1 @@ -04db2d972cb7ea3fc0ec9aea4993e33ddbf22a77 +8563add3b7f509de3b49ba04863d2708d965b489 diff --git a/boilerplate/_lib/boilerplate-commit b/boilerplate/_lib/boilerplate-commit index 074f74f4..5ec2a77b 100755 --- a/boilerplate/_lib/boilerplate-commit +++ b/boilerplate/_lib/boilerplate-commit @@ -1,6 +1,9 @@ #!/usr/bin/env bash set -e +if [ "$BOILERPLATE_SET_X" ]; then + set -x +fi REPO_ROOT=$(git rev-parse --show-toplevel) source $REPO_ROOT/boilerplate/_lib/common.sh @@ -53,8 +56,14 @@ elif grep -q '^ M boilerplate/_data/last-boilerplate-commit$' $git_status; then bp_compare_url="https://github.com/openshift/boilerplate/compare/$bp_commit_change" # Generate the commit history for this range. This will go in the commit message. ( - $BOILERPLATE_GIT_CLONE $bp_clone - cd $bp_clone + if [[ -z "${BOILERPLATE_IN_CI}" ]]; then + git clone "${BOILERPLATE_GIT_REPO}" "${bp_clone}" + else + # HACK: We can't get around safe.directory in CI, so just leverage cp instead of git + cp -r /go/src/github.com/openshift/boilerplate/* "${bp_clone}" + cp -r /go/src/github.com/openshift/boilerplate/.git "${bp_clone}" + fi + cd "${bp_clone}" # Matches promote.sh git log --no-merges --pretty=format:'commit: %H%nauthor: %an%n%s%n%n%b%n%n' $bp_commit_change > $bp_log ) diff --git a/boilerplate/_lib/common.sh b/boilerplate/_lib/common.sh index 78703f61..c5ee7f4d 100755 --- a/boilerplate/_lib/common.sh +++ b/boilerplate/_lib/common.sh @@ -107,12 +107,12 @@ image_exists_in_repo() { fi echo "Image ${image_uri} exists with digest $digest." return 0 - elif [[ "$stderr" == *"manifest unknown"* ]]; then + elif [[ "$output" == *"manifest unknown"* || "$stderr" == *"manifest unknown"* ]]; then # We were able to talk to the repository, but the tag doesn't exist. # This is the normal "green field" case. echo "Image ${image_uri} does not exist in the repository." return 1 - elif [[ "$stderr" == *"was deleted or has expired"* ]]; then + elif [[ "$output" == *"manifest unknown"* || "$stderr" == *"was deleted or has expired"* ]]; then # This should be rare, but accounts for cases where we had to # manually delete an image. echo "Image ${image_uri} was deleted from the repository." @@ -177,9 +177,6 @@ fi if [ -z "$BOILERPLATE_GIT_REPO" ]; then export BOILERPLATE_GIT_REPO=https://github.com/openshift/boilerplate.git fi -if [ -z "$BOILERPLATE_GIT_CLONE" ]; then - export BOILERPLATE_GIT_CLONE="git clone $BOILERPLATE_GIT_REPO" -fi # The namespace of the ImageStream by which prow will import the image. IMAGE_NAMESPACE=openshift diff --git a/boilerplate/_lib/freeze-check b/boilerplate/_lib/freeze-check index 080629f5..4109ac9a 100755 --- a/boilerplate/_lib/freeze-check +++ b/boilerplate/_lib/freeze-check @@ -67,7 +67,7 @@ git reset --hard FETCH_HEAD # close a security hole whereby the latter is overridden. echo "Running update" cd $REPO_ROOT -BOILERPLATE_GIT_CLONE="git clone $TMPD" boilerplate/update +BOILERPLATE_GIT_REPO="${TMPD}" boilerplate/update # Okay, if anything has changed, that's bad. if [[ $(git status --porcelain -- ':!build/Dockerfile*' | wc -l) -ne 0 ]]; then diff --git a/boilerplate/openshift/golang-lint/ensure.sh b/boilerplate/openshift/golang-lint/ensure.sh index 1b2b79ea..d37cb863 100755 --- a/boilerplate/openshift/golang-lint/ensure.sh +++ b/boilerplate/openshift/golang-lint/ensure.sh @@ -5,7 +5,7 @@ set -eo pipefail REPO_ROOT=$(git rev-parse --show-toplevel) source $REPO_ROOT/boilerplate/_lib/common.sh -GOLANGCI_LINT_VERSION="1.54.2" +GOLANGCI_LINT_VERSION="1.59.1" DEPENDENCY=${1:-} GOOS=$(go env GOOS) diff --git a/boilerplate/openshift/golang-lint/golangci.yml b/boilerplate/openshift/golang-lint/golangci.yml index 7265cf65..b489f3ba 100644 --- a/boilerplate/openshift/golang-lint/golangci.yml +++ b/boilerplate/openshift/golang-lint/golangci.yml @@ -17,9 +17,17 @@ linters: disable-all: true enable: - errcheck + - gosec - gosimple - govet - ineffassign + - misspell - staticcheck - typecheck - unused + +linters-settings: + misspell: + extra-words: + - typo: "openshit" + correction: "OpenShift" diff --git a/boilerplate/update b/boilerplate/update index fb48c57b..9a4aa98b 100755 --- a/boilerplate/update +++ b/boilerplate/update @@ -11,7 +11,7 @@ set -e if [ "$BOILERPLATE_SET_X" ]; then - set -x + set -x fi # The directory in which this script lives is the CONVENTION_ROOT. Export @@ -105,10 +105,17 @@ EOF BOILERPLATE_GIT_REPO=git@github.com:openshift/boilerplate.git fi if [ -z "$BOILERPLATE_GIT_CLONE" ]; then - BOILERPLATE_GIT_CLONE="git clone $BOILERPLATE_GIT_REPO" + BOILERPLATE_GIT_CLONE="git clone" fi BP_CLONE="$(mktemp -d)" - $BOILERPLATE_GIT_CLONE "${BP_CLONE}" + + if [[ -z "${BOILERPLATE_IN_CI}" ]]; then + ${BOILERPLATE_GIT_CLONE} "${BOILERPLATE_GIT_REPO}" "${BP_CLONE}" + else + # HACK: We can't get around safe.directory in CI, so just leverage cp instead of git + cp -rf /go/src/github.com/openshift/boilerplate/* "${BP_CLONE}" + cp -rf /go/src/github.com/openshift/boilerplate/.git "${BP_CLONE}" + fi echo "Updating the update script." rsync -a "${BP_CLONE}/boilerplate/update" "$0" echo "Copying utilities" @@ -159,7 +166,9 @@ if [ ! -f "$CONFIG_FILE" ]; then fi # The most recent build image tag. Export this for individual `update` scripts. -export LATEST_IMAGE_TAG=$(cd $BP_CLONE; git describe --tags --abbrev=0 --match image-v*) +if [[ -z "$LATEST_IMAGE_TAG" ]]; then + export LATEST_IMAGE_TAG=$(cd $BP_CLONE; git describe --tags --abbrev=0 --match image-v*) +fi # Prepare the "nexus makefile include". NEXUS_MK="${CONVENTION_ROOT}/generated-includes.mk" diff --git a/cmd/egress/cmd.go b/cmd/egress/cmd.go index abe0bdf2..fc3c5fe0 100644 --- a/cmd/egress/cmd.go +++ b/cmd/egress/cmd.go @@ -248,7 +248,7 @@ are set correctly before execution. validateEgressCmd.Flags().StringSliceVar(&config.securityGroupIDs, "security-group-ids", []string{}, "(optional) comma-separated list of sec. group IDs to attach to the created EC2 instance. If absent, one will be created") validateEgressCmd.Flags().StringVar(&config.egressListLocation, "egress-list-location", "", "(optional) the location of the egress URL list to use. Can either be a local file path or an external URL starting with http(s). This value is ignored for the legacy probe.") validateEgressCmd.Flags().StringVar(&config.region, "region", "", fmt.Sprintf("(optional) compute instance region. If absent, environment var %[1]v = %[2]v and %[3]v = %[4]v will be used", awsRegionEnvVarStr, awsRegionDefault, gcpRegionEnvVarStr, gcpRegionDefault)) - validateEgressCmd.Flags().StringToStringVar(&config.cloudTags, "cloud-tags", map[string]string{}, "(optional) comma-seperated list of tags to assign to cloud resources e.g. --cloud-tags key1=value1,key2=value2") + validateEgressCmd.Flags().StringToStringVar(&config.cloudTags, "cloud-tags", map[string]string{}, "(optional) comma-separated list of tags to assign to cloud resources e.g. --cloud-tags key1=value1,key2=value2") validateEgressCmd.Flags().BoolVar(&config.debug, "debug", false, "(optional) if true, enable additional debug-level logging") validateEgressCmd.Flags().DurationVar(&config.timeout, "timeout", time.Duration(0), "(optional) timeout for individual egress verification requests") validateEgressCmd.Flags().StringVar(&config.kmsKeyID, "kms-key-id", "", "(optional) ID of KMS key used to encrypt root volumes of compute instances. Defaults to cloud account default key") @@ -256,7 +256,7 @@ are set correctly before execution. validateEgressCmd.Flags().StringVar(&config.httpsProxy, "https-proxy", "", "(optional) https-proxy to be used upon https requests being made by verifier, format: https://user:pass@x.x.x.x:8978") validateEgressCmd.Flags().StringVar(&config.CaCert, "cacert", "", "(optional) path to cacert file to be used upon https requests being made by verifier") validateEgressCmd.Flags().BoolVar(&config.noTls, "no-tls", false, "(optional) if true, skip client-side SSL certificate validation") - validateEgressCmd.Flags().StringSliceVar(&config.noProxy, "no-proxy", []string{}, "(optional) comma-seperated list of domains or IPs to not pass through the configured http/https proxy e.g. --no-proxy example.com,test.example.com") + validateEgressCmd.Flags().StringSliceVar(&config.noProxy, "no-proxy", []string{}, "(optional) comma-separated list of domains or IPs to not pass through the configured http/https proxy e.g. --no-proxy example.com,test.example.com") validateEgressCmd.Flags().StringVar(&config.awsProfile, "profile", "", "(optional) AWS profile. If present, any credentials passed with CLI will be ignored") validateEgressCmd.Flags().StringVar(&config.gcpVpcName, "vpc-name", "", "(optional unless --platform='gcp') VPC name where GCP cluster is installed") validateEgressCmd.Flags().BoolVar(&config.skipAWSInstanceTermination, "skip-termination", false, "(optional) Skip instance termination to allow further debugging") @@ -321,12 +321,18 @@ func getCustomLocalEgressList(filePath string) (string, error) { return string(file), nil } -func getCustomExternalEgressList(url string) (string, error) { - response, err := http.Get(url) +func getCustomExternalEgressList(uri string) (string, error) { + req, err := http.NewRequest(http.MethodGet, uri, nil) if err != nil { return "", err } - b, err := io.ReadAll(response.Body) + + res, err := http.DefaultClient.Do(req) + if err != nil { + return "", err + } + + b, err := io.ReadAll(res.Body) if err != nil { return "", err } diff --git a/pkg/data/cloud/platform_test.go b/pkg/data/cloud/platform_test.go index 9a54c62f..6f2f672f 100644 --- a/pkg/data/cloud/platform_test.go +++ b/pkg/data/cloud/platform_test.go @@ -13,43 +13,43 @@ func TestPlatform_Comparable(t *testing.T) { func TestPlatform_String(t *testing.T) { tests := []struct { name string - platfrom Platform + platform Platform want string }{ { name: "aws", - platfrom: AWSClassic, + platform: AWSClassic, want: "aws-classic", }, { name: "aws-classic", - platfrom: AWSClassic, + platform: AWSClassic, want: "aws-classic", }, { name: "hosted-cluster", - platfrom: AWSHCP, + platform: AWSHCP, want: "aws-hcp", }, { name: "aws-hcp", - platfrom: AWSHCP, + platform: AWSHCP, want: "aws-hcp", }, { name: "gcp", - platfrom: GCPClassic, + platform: GCPClassic, want: "gcp-classic", }, { name: "gcp-classic", - platfrom: GCPClassic, + platform: GCPClassic, want: "gcp-classic", }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - platform := tt.platfrom + platform := tt.platform if got := platform.String(); got != tt.want { t.Errorf("Platform.String() = %s, want %s", got, tt.want) } diff --git a/pkg/helpers/helpers.go b/pkg/helpers/helpers.go index bb6d9d47..9f4afb68 100644 --- a/pkg/helpers/helpers.go +++ b/pkg/helpers/helpers.go @@ -1,9 +1,10 @@ package helpers import ( + "crypto/rand" "errors" "fmt" - "math/rand" + "math/big" "regexp" "strconv" "strings" @@ -13,13 +14,23 @@ import ( ) // RandSeq generates random string with n characters. -func RandSeq(n int) string { +func RandSeq(n int) (string, error) { b := make([]rune, n) var letters = []rune("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ") + for i := range b { - b[i] = letters[rand.Intn(len(letters))] + num, err := rand.Int(rand.Reader, big.NewInt(int64(len(letters)))) + if err != nil { + return "", err + } + b[i] = letters[num.Int64()] } - return string(b) + return string(b), nil +} + +// RandBigInt returns a pointer to a random big integer between 0 and max +func RandBigInt(max int64) (*big.Int, error) { + return rand.Int(rand.Reader, big.NewInt(max)) } // PollImmediate calls the condition function at the specified interval up to the specified timeout @@ -170,7 +181,7 @@ func ValidateProvidedVariables(providedVarMap map[string]string, presetVarMap ma // CutBetween returns the part of s between startingToken and endingToken. If startingToken and/or // endingToken cannot be found in s, or if there are no characters between the two tokens, this -// returns an empty string (""). If there are multiple occurrances of each token, the largest possible +// returns an empty string (""). If there are multiple occurrences of each token, the largest possible // part of s will be returned (i.e., everything between the leftmost startingToken and the rightmost // endingToken, a.k.a. greedy matching) func CutBetween(s string, startingToken string, endingToken string) string { diff --git a/pkg/helpers/helpers_test.go b/pkg/helpers/helpers_test.go index c486fa03..0d750e20 100644 --- a/pkg/helpers/helpers_test.go +++ b/pkg/helpers/helpers_test.go @@ -2,6 +2,7 @@ package helpers import ( _ "embed" + "fmt" "reflect" "testing" @@ -635,3 +636,21 @@ func TestDurationToBareSeconds(t *testing.T) { }) } } + +func Test_RandSeq(t *testing.T) { + val, err := RandSeq(10) + fmt.Print(val) + if err != nil { + t.Fatal(err) + } + if len(val) != 10 { + t.Errorf("len(val) = %d, want 10", len(val)) + } +} + +func Test_RandBigInt(t *testing.T) { + _, err := RandBigInt(1000) + if err != nil { + t.Fatal(err) + } +} diff --git a/pkg/verifier/aws/aws_verifier.go b/pkg/verifier/aws/aws_verifier.go index f40ed9dc..910e350a 100644 --- a/pkg/verifier/aws/aws_verifier.go +++ b/pkg/verifier/aws/aws_verifier.go @@ -455,8 +455,13 @@ func (a *AwsVerifier) writeDebugLogs(ctx context.Context, log string) { // CreateSecurityGroup creates a security group with the specified name and cluster tag key in a specified VPC func (a *AwsVerifier) CreateSecurityGroup(ctx context.Context, tags map[string]string, name, vpcId string) (*ec2.CreateSecurityGroupOutput, error) { + seq, err := helpers.RandSeq(5) + if err != nil { + return &ec2.CreateSecurityGroupOutput{}, err + } + input := &ec2.CreateSecurityGroupInput{ - GroupName: awsTools.String(name + "-" + helpers.RandSeq(5)), + GroupName: awsTools.String(name + "-" + seq), VpcId: &vpcId, Description: awsTools.String("osd-network-verifier security group"), TagSpecifications: []ec2Types.TagSpecification{ diff --git a/pkg/verifier/gcp/entry_point.go b/pkg/verifier/gcp/entry_point.go index 5103ffbf..cdc699f2 100644 --- a/pkg/verifier/gcp/entry_point.go +++ b/pkg/verifier/gcp/entry_point.go @@ -3,7 +3,7 @@ package gcpverifier import ( "encoding/base64" "fmt" - "math/rand" + "github.com/openshift/osd-network-verifier/pkg/helpers" "strconv" "time" @@ -110,6 +110,12 @@ func (g *GcpVerifier) ValidateEgress(vei verifier.ValidateEgressInput) *output.O } } + // Generate a random integer to be used in the instanceName + randInt, err := helpers.RandBigInt(10000) + if err != nil { + return g.Output.AddError(err) + } + // Create the ComputeService instance instance, err := g.createComputeServiceInstance(createComputeServiceInstanceInput{ projectID: vei.GCP.ProjectID, @@ -117,7 +123,7 @@ func (g *GcpVerifier) ValidateEgress(vei verifier.ValidateEgressInput) *output.O vpcSubnetID: fmt.Sprintf("projects/%s/regions/%s/subnetworks/%s", vei.GCP.ProjectID, vei.GCP.Region, vei.SubnetID), userdata: userData, machineType: vei.InstanceType, - instanceName: fmt.Sprintf("verifier-%v", rand.Intn(10000)), + instanceName: fmt.Sprintf("verifier-%v", randInt.String()), sourceImage: fmt.Sprintf("projects/rhel-cloud/global/images/%s", vei.CloudImageID), networkName: fmt.Sprintf("projects/%s/global/networks/%s", vei.GCP.ProjectID, vei.GCP.VpcName), tags: vei.Tags,