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

Generate & Publish based on tf provider fork #204

Conversation

denniskniep
Copy link
Contributor

Description of your changes

For development purposes it is useful to be able to generate and publish on top of a forked terraform provider.
Therefore this PR adds:

  • Download tf-provider from github releases (instead of from hashicorp tf repo)
  • Use optional action vars for ci to specify tf provider source (upstream or fork)
  • Publish to different repo via optional action vars in ci

Please see Readme.md for more details.

BREAKING CHANGE:
The Github Action secret UPBOUND_MARKETPLACE_PUSH_ROBOT_USR needs to be moved to a Github Action variable

I have:

  • Read and followed Crossplane's [contribution process].
  • Run make reviewable test to ensure this PR is ready for review.

@Breee
Copy link
Collaborator

Breee commented Jan 12, 2025

wrt to UPBOUND_MARKETPLACE_PUSH_ROBOT_USR that has to be done by some admin of the repo.

e.g. @haarchri

As i cannot see or change the secrets / env vars

@denniskniep
Copy link
Contributor Author

@Breee what do you think in general about that change? Do you think it is useful to include it ?

@haarchri
Copy link
Member

I would avoid this Change:

The Github Action secret UPBOUND_MARKETPLACE_PUSH_ROBOT_USR needs to be moved to a Github Action variable

all other providers in eco-system have a configuration to get this from secrets - why it's not possible to leave it as is ?

@Breee
Copy link
Collaborator

Breee commented Jan 12, 2025

Thinking more about it, I don't think it's necessary.

Currently one can easily overwrite all the variables he needs and publish to their own registry using make publish.all (e,g, https://gitlab.com/corewire/images/provider-keycloak/-/blob/main/build.sh?ref_type=heads#L66, https://gitlab.com/corewire/images/provider-keycloak/-/blob/main/build.sh?ref_type=heads#L104)

We could provide a command in the makefile / a script tho to make this as simple as possible.

What is the benefit for you @denniskniep to publish via a CI/CD workflow instead of the makefile while developing?

@denniskniep
Copy link
Contributor Author

Ok, its not absolutely necessary for me to integrate this with CI, though it would be nice, because I want to use it for temp. deployments if certain changes are not quickly integrated upstream.

Therefore also the change wrt UPBOUND_MARKETPLACE_PUSH_ROBOT_USR is not necessary anymore. (The change was necessary, because everything like env vars, which has the same value as the secret is masked automatically)

But can we agree on, that we stick with the new approach to download tf-provider from github releases (instead of from hashicorp tf repo)?

Then I will change the PR accordingly and also the readme part how to use a custom tf-provider fork and how to publish via makefile

@Breee
Copy link
Collaborator

Breee commented Jan 12, 2025 via email

@denniskniep denniskniep force-pushed the feature/release-based-on-tf-provider-fork branch from 6a42f87 to 3114de7 Compare January 29, 2025 10:23
    * Download tf-provider from github releases
    * Document how to publish to different repo

Signed-off-by: Dennis Kniep <[email protected]>
@denniskniep denniskniep force-pushed the feature/release-based-on-tf-provider-fork branch from 3114de7 to 28a1d30 Compare January 29, 2025 10:25
@denniskniep
Copy link
Contributor Author

@Breee
rebased to main and reverted the CI Pipeline changes. Added documentation how to publish from local system.

@Breee
Copy link
Collaborator

Breee commented Jan 29, 2025

lgtm

@Breee Breee merged commit 58d214b into crossplane-contrib:main Jan 29, 2025
6 checks passed
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.

3 participants