Skip to content

Commit

Permalink
Update boilerplate, fix linting issues (#290)
Browse files Browse the repository at this point in the history
* Update boilerplate

* Fixes for newly failing linter rules
  • Loading branch information
joshbranham authored Dec 2, 2024
1 parent 25b8154 commit 8a6139a
Show file tree
Hide file tree
Showing 15 changed files with 106 additions and 37 deletions.
1 change: 0 additions & 1 deletion OWNERS
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ reviewers:
- abyrne55
- reedcort
- dakotalongRH
- lnguyen1401
- luis-falcon
- joshbranham
approvers:
Expand Down
2 changes: 1 addition & 1 deletion boilerplate/_data/backing-image-tag
Original file line number Diff line number Diff line change
@@ -1 +1 @@
image-v5.0.0
image-v6.0.1
2 changes: 1 addition & 1 deletion boilerplate/_data/last-boilerplate-commit
Original file line number Diff line number Diff line change
@@ -1 +1 @@
04db2d972cb7ea3fc0ec9aea4993e33ddbf22a77
8563add3b7f509de3b49ba04863d2708d965b489
13 changes: 11 additions & 2 deletions boilerplate/_lib/boilerplate-commit
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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
)
Expand Down
7 changes: 2 additions & 5 deletions boilerplate/_lib/common.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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."
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion boilerplate/_lib/freeze-check
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion boilerplate/openshift/golang-lint/ensure.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
8 changes: 8 additions & 0 deletions boilerplate/openshift/golang-lint/golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
17 changes: 13 additions & 4 deletions boilerplate/update
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down
16 changes: 11 additions & 5 deletions cmd/egress/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -248,15 +248,15 @@ 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")
validateEgressCmd.Flags().StringVar(&config.httpProxy, "http-proxy", "", "(optional) http-proxy to be used upon http requests being made by verifier, format: http://user:pass@x.x.x.x:8978")
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")
Expand Down Expand Up @@ -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
}
Expand Down
16 changes: 8 additions & 8 deletions pkg/data/cloud/platform_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
21 changes: 16 additions & 5 deletions pkg/helpers/helpers.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
package helpers

import (
"crypto/rand"
"errors"
"fmt"
"math/rand"
"math/big"
"regexp"
"strconv"
"strings"
Expand All @@ -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
Expand Down Expand Up @@ -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 {
Expand Down
19 changes: 19 additions & 0 deletions pkg/helpers/helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package helpers

import (
_ "embed"
"fmt"
"reflect"
"testing"

Expand Down Expand Up @@ -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)
}
}
7 changes: 6 additions & 1 deletion pkg/verifier/aws/aws_verifier.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
10 changes: 8 additions & 2 deletions pkg/verifier/gcp/entry_point.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ package gcpverifier
import (
"encoding/base64"
"fmt"
"math/rand"
"github.com/openshift/osd-network-verifier/pkg/helpers"
"strconv"
"time"

Expand Down Expand Up @@ -110,14 +110,20 @@ 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,
zone: vei.GCP.Zone,
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,
Expand Down

0 comments on commit 8a6139a

Please sign in to comment.