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: Remove indirect import of AWS package #1585

Conversation

mateusoliveira43
Copy link
Contributor

Why the changes were made

While working on migtools/oadp-non-admin#37, I will need to add oadp-operator as a import in oadp-non-admin. In my first test, while I just need to import api/v1alpha1, I noticed github.com/aws/aws-sdk-go was being added to oadp-non-admin's go.mod. On inspecting I saw this:

github.com/aws/aws-sdk-go
This module is necessary because github.com/aws/aws-sdk-go/aws/request is imported in:

- github.com/migtools/oadp-non-admin/cmd
-- github.com/openshift/oadp-operator/api/v1alpha1
--- github.com/openshift/oadp-operator/pkg/common

This PR removes github.com/aws/aws-sdk-go from pkg/common

How to test the changes made

Check that this change did not break any part.

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 8, 2024
Copy link
Member

@kaovilai kaovilai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This lgtm

Copy link

openshift-ci bot commented Nov 8, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kaovilai, mateusoliveira43, shubham-pampattiwar

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [kaovilai,mateusoliveira43,shubham-pampattiwar]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Copy link
Member

@kaovilai kaovilai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

validated in codespaces

@kaovilai ➜ /workspaces/oadp-operator (fix/remove-indirect-aws-import) $ go list --deps ./pkg/common   
internal/goarch
unsafe
internal/abi
internal/unsafeheader
internal/cpu
internal/bytealg
internal/byteorder
internal/chacha8rand
internal/coverage/rtcov
internal/godebugs
internal/goexperiment
internal/goos
internal/profilerecord
internal/runtime/atomic
internal/runtime/exithook
internal/runtime/syscall
internal/stringslite
runtime/internal/math
runtime/internal/sys
runtime
internal/reflectlite
errors
cmp
internal/itoa
internal/race
iter
math/bits
math
unicode/utf8
strconv
sync/atomic
sync
unicode
reflect
slices
internal/fmtsort
io
internal/oserror
path
internal/bisect
internal/godebug
internal/asan
internal/msan
syscall
time
io/fs
internal/filepathlite
internal/syscall/unix
internal/poll
internal/syscall/execenv
internal/testlog
os
fmt
strings
github.com/vmware-tanzu/velero/pkg/types
bytes
bufio
encoding
encoding/binary
encoding/base64
unicode/utf16
encoding/json
log/internal
log
sort
github.com/gogo/protobuf/proto
github.com/gogo/protobuf/sortkeys
math/rand
math/big
gopkg.in/inf.v0
k8s.io/apimachinery/pkg/api/resource
github.com/google/gofuzz/bytesource
regexp/syntax
regexp
github.com/google/gofuzz
k8s.io/apimachinery/third_party/forked/golang/reflect
k8s.io/apimachinery/pkg/conversion
k8s.io/apimachinery/pkg/selection
k8s.io/apimachinery/pkg/fields
k8s.io/apimachinery/pkg/util/sets
k8s.io/apimachinery/pkg/util/errors
k8s.io/apimachinery/pkg/util/validation/field
context
vendor/golang.org/x/net/dns/dnsmessage
internal/nettrace
internal/singleflight
math/rand/v2
internal/concurrent
internal/weak
unique
net/netip
runtime/cgo
net
k8s.io/utils/internal/third_party/forked/golang/net
k8s.io/utils/net
k8s.io/apimachinery/pkg/util/validation
flag
log/slog/internal
log/slog/internal/buffer
log/slog
github.com/go-logr/logr
github.com/go-logr/logr/slogr
k8s.io/klog/v2/internal/severity
k8s.io/klog/v2/internal/buffer
k8s.io/klog/v2/internal/clock
k8s.io/klog/v2/internal/dbg
k8s.io/klog/v2/internal/serialize
k8s.io/klog/v2/internal/sloghandler
os/user
path/filepath
k8s.io/klog/v2
k8s.io/utils/strings/slices
k8s.io/apimachinery/pkg/labels
go/token
go/scanner
go/ast
go/doc/comment
internal/lazyregexp
go/doc
go/build/constraint
go/internal/typeparams
go/parser
net/url
k8s.io/apimachinery/pkg/conversion/queryparams
k8s.io/apimachinery/pkg/runtime/schema
sigs.k8s.io/json/internal/golang/encoding/json
sigs.k8s.io/json
k8s.io/apimachinery/pkg/util/json
runtime/debug
k8s.io/apimachinery/pkg/util/naming
compress/flate
hash
hash/crc32
compress/gzip
container/list
crypto
crypto/internal/alias
crypto/subtle
crypto/cipher
crypto/internal/boring/sig
crypto/internal/boring
crypto/internal/randutil
crypto/rand
crypto/aes
crypto/des
crypto/internal/edwards25519/field
crypto/internal/nistec/fiat
embed
crypto/internal/nistec
crypto/ecdh
crypto/elliptic
crypto/internal/bigmod
crypto/internal/boring/bbig
crypto/sha512
encoding/asn1
vendor/golang.org/x/crypto/cryptobyte/asn1
vendor/golang.org/x/crypto/cryptobyte
crypto/ecdsa
crypto/internal/edwards25519
crypto/ed25519
crypto/hmac
vendor/golang.org/x/crypto/internal/alias
vendor/golang.org/x/crypto/chacha20
vendor/golang.org/x/crypto/internal/poly1305
vendor/golang.org/x/sys/cpu
vendor/golang.org/x/crypto/chacha20poly1305
vendor/golang.org/x/crypto/hkdf
crypto/internal/hpke
vendor/golang.org/x/crypto/sha3
crypto/internal/mlkem768
crypto/md5
crypto/rc4
crypto/rsa
crypto/sha1
crypto/sha256
crypto/dsa
encoding/hex
crypto/x509/pkix
encoding/pem
crypto/x509
crypto/tls
vendor/golang.org/x/text/transform
vendor/golang.org/x/text/unicode/bidi
vendor/golang.org/x/text/secure/bidirule
vendor/golang.org/x/text/unicode/norm
vendor/golang.org/x/net/idna
net/textproto
vendor/golang.org/x/net/http/httpguts
vendor/golang.org/x/net/http/httpproxy
vendor/golang.org/x/net/http2/hpack
maps
mime
mime/quotedprintable
mime/multipart
net/http/httptrace
net/http/internal
net/http/internal/ascii
net/http
k8s.io/apimachinery/pkg/util/runtime
io/ioutil
github.com/modern-go/concurrent
github.com/modern-go/reflect2
github.com/json-iterator/go
gopkg.in/yaml.v2
sigs.k8s.io/structured-merge-diff/v4/value
k8s.io/apimachinery/pkg/runtime
k8s.io/apimachinery/pkg/types
k8s.io/apimachinery/pkg/util/intstr
golang.org/x/text/transform
golang.org/x/text/unicode/bidi
golang.org/x/text/secure/bidirule
golang.org/x/text/unicode/norm
golang.org/x/net/idna
golang.org/x/net/http/httpguts
golang.org/x/net/http2/hpack
golang.org/x/net/http2
k8s.io/apimachinery/pkg/util/net
k8s.io/apimachinery/pkg/watch
k8s.io/apimachinery/pkg/apis/meta/v1
k8s.io/api/core/v1
github.com/openshift/oadp-operator/pkg/common
@kaovilai ➜ /workspaces/oadp-operator (fix/remove-indirect-aws-import) $ go list --deps ./pkg/common | grep aws
@kaovilai ➜ /workspaces/oadp-operator (fix/remove-indirect-aws-import) $ 

@kaovilai
Copy link
Member

kaovilai commented Nov 8, 2024

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Nov 8, 2024
@mateusoliveira43
Copy link
Contributor Author

/test 4.15-e2e-test-aws
restore works, but app does not get ready flake

@openshift-ci-robot
Copy link

/retest-required

Remaining retests: 0 against base HEAD dd2ddc9 and 2 for PR HEAD 466f1b2 in total

2 similar comments
@openshift-ci-robot
Copy link

/retest-required

Remaining retests: 0 against base HEAD dd2ddc9 and 2 for PR HEAD 466f1b2 in total

@openshift-ci-robot
Copy link

/retest-required

Remaining retests: 0 against base HEAD dd2ddc9 and 2 for PR HEAD 466f1b2 in total

@kaovilai
Copy link
Member

/retest-required

@openshift-ci-robot
Copy link

/retest-required

Remaining retests: 0 against base HEAD dd2ddc9 and 2 for PR HEAD 466f1b2 in total

@kaovilai
Copy link
Member

/override ci/prow/4.17-e2e-test-kubevirt-aws

@kaovilai
Copy link
Member

Waiting for volume binding

Copy link

openshift-ci bot commented Nov 11, 2024

@kaovilai: Overrode contexts on behalf of kaovilai: ci/prow/4.17-e2e-test-kubevirt-aws

In response to this:

/override ci/prow/4.17-e2e-test-kubevirt-aws

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@openshift-merge-bot openshift-merge-bot bot merged commit 738a1a0 into openshift:master Nov 11, 2024
22 checks passed
Copy link

openshift-ci bot commented Nov 11, 2024

@mateusoliveira43: all tests passed!

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants