-
Notifications
You must be signed in to change notification settings - Fork 41
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
Updates dependencies to get us pretty close to current #336
Conversation
Signed-off-by: Taylor Thornton <thornton.tn@gmail.com>
Signed-off-by: Taylor Thornton <thornton.tn@gmail.com>
Signed-off-by: Taylor Thornton <thornton.tn@gmail.com>
- update vendored packages to meet new functionality from upstream Signed-off-by: Taylor Thornton <thornton.tn@gmail.com>
Signed-off-by: Taylor Thornton <thornton.tn@gmail.com>
Signed-off-by: Taylor Thornton <thornton.tn@gmail.com>
cmd/up/main.go
Outdated
// We don't need this choice to be cryptographically random. | ||
rand.New(rand.NewSource(time.Now().UTC().UnixNano())) // nolint:gosec |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just because I'm curious: If we don't need it to be cryptographically strong, why do we need random? What do we use it for?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Funny enough, in searching for the answer to your question it looks like it was being used for some now dead code: c4c3627. I just removed this line to remove the confusion.
Signed-off-by: Taylor Thornton <thornton.tn@gmail.com>
// The following replacements should be kept in sync with upstream | ||
// Kubernetes version. | ||
// xref: https://github.com/kubernetes/kubernetes/blob/4ce5a8954017644c5420bae81d72b09b735c21f0/go.mod#L394 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yay! I'm happy to see these replacements removed.
Signed-off-by: Taylor Thornton <thornton.tn@gmail.com>
// https://github.com/helm/helm/issues/11821 | ||
github.com/docker/cli => github.com/docker/cli v20.10.19+incompatible | ||
github.com/docker/docker => github.com/docker/docker v20.10.19+incompatible |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ewww.
I know it's not your fault and it looks like we have to do this, but still. Eww.
LGTM! I'll wait to approve until you test, but it looks reasonable. My only concern is about what happens for new releases of Crossplane. Will it be a lot of work to update the vendored code? Would it help to document anything about the process you followed to do the update, or is just something that we need to work through? |
It's incredibly unclear what the LOE is to update the next time around. Regarding documentation, I don't really know what to document:
|
OK, that's cool. No need to do anything to document it. |
Should we do a new release of up in the near future? |
That makes sense to me. Maybe sometime next week so that we can at the very least get fixes for #329 out. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @tnthornton ! Did a few more tests with xpkg xp-extract
with a local build and looks good 👍🏼
@phisco did you want this backported?
@jastang I don't think we support multiple releases, looking at https://github.com/upbound/up/tags it looks like we are always going forward with versions, so nothing to backport to |
Description of your changes
It's been a long time since dependencies were updated in this repo. Some of the consequences of that are:
The overarching goal of this PR is to get us roughly caught up on the crossplane/crossplane dependency which results in a cascade affect of the other dependencies needing to be updated.
In addition:
Fixes #329
I have:
make reviewable
to ensure this PR is ready for review.How has this code been tested
make build test reviewable
Testing xpls
make build
See below screenshots, prior functionality appears to continue to work
Testing xpkg commands
a. Checking if #329 is resolved
ezgi-platform-ref-gcp.xpkg
file.up xpkg xp-extract --from-xpkg ezgi-platform-ref-gcp.xpkg
gzip --uncompress out.gz
out
file, then searched forstatus.atProvider.id
and found:b. Verified that I can push xpkg to registry
c. Verified that I can pull that xpkg into a control plane