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 cert-manager's SDK location #1112

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix cert-manager's SDK location #1112

wants to merge 1 commit into from

Conversation

blampe
Copy link
Contributor

@blampe blampe commented Oct 29, 2024

Copy link
Member

@danielrbradley danielrbradley left a comment

Choose a reason for hiding this comment

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

Is the SDK actually already published on the same tag as the root repo which triggers the release? At which point, how do we embed the version numbers into the Go SDK?

I.e. if the Go mod lives in the root, then pushing the tag v1.2.3 will essentially create a release of the SDK. When this action runs and attempts to push the the Go SDK with the version number on a standalone commit, I think it'll try and push the tag v1.2.3 to that new commit too, which will fail because it already exists.

@blampe
Copy link
Contributor Author

blampe commented Oct 31, 2024

I.e. if the Go mod lives in the root, then pushing the tag v1.2.3 will essentially create a release of the SDK. When this action runs and attempts to push the the Go SDK with the version number on a standalone commit, I think it'll try and push the tag v1.2.3 to that new commit too, which will fail because it already exists.

That's a good point... what do you think would be the best way to handle this? Do we not need the publish SDK step?

@danielrbradley
Copy link
Member

I think the pragmatic solution is to disable the Go publish step for this provider for now.

This publishing method likely means the SDK is either not pinning the provider version or is pinning the wrong provider version. We should open an issue to investigate. It's possible that the only way to make this work right now is to hard code the version in the repo and bump it ahead of each release.

Long term, we should use the same design of SDK publishing for consistency but that will be a breaking change.

@rshade
Copy link
Contributor

rshade commented Dec 18, 2024

@blampe this is going to affect ingress-nginx as well now, can I push a commit to this branch to fix those as well?

rshade added a commit that referenced this pull request Dec 20, 2024
@blampe
Copy link
Contributor Author

blampe commented Dec 20, 2024

@rshade sorry I missed this, I think Daniel's suggestion to disable the Go publishing step is probably the best path forward for now, but that will probably requiring a little bit of plumbing in ci-mgmt.

@rshade
Copy link
Contributor

rshade commented Dec 20, 2024

@blampe no worries, I made a PR instead. I did try to make a go example on nginx and ran into problems:

go mod tidy
go: finding module for package github.com/pulumi/pulumi-kubernetes-ingress-nginx/sdk
go: simple-nginx-go imports
        github.com/pulumi/pulumi-kubernetes-ingress-nginx/sdk: module github.com/pulumi/pulumi-kubernetes-ingress-nginx/sdk@latest found (v0.0.10, replaced by ../../sdk), but does not contain package github.com/pulumi/pulumi-kubernetes-ingress-nginx/sdk

I am not sure that go.mod is setup right in sdk on either repo

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Workflow failure: release
3 participants