-
Notifications
You must be signed in to change notification settings - Fork 61
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
WIP: add support for Oberve Only resources #167
Conversation
This adds the new ManagementPolicy field to the Workspace resource by way of updating the 'crossplane-runtime' package to the latest trunk branch version. This also resolves a dependency conflict if anyone imports both the API types from this module and the AWS provider, which was already using the newer version. Related dependencies were also updated. Signed-off-by: Josh Feierman <[email protected]>
This updates the version of golangci-lint used. The previous version (1.50.0) had performance issues that prevented the lint command from completing on my local machine. Signed-off-by: Josh Feierman <[email protected]>
This adds the required changes to the internal/controller/config package due to changes in the upstream controller-runtime API.
If the flag to enable Management Policies is set, the Reconciler is now passed the equivalent option.
@yardbirdsax that is interesting. How do you envision Observe Only resources working in combination with provider-terraform Workspace? What is the desired result if we compare with the execution of standard terraform |
Good questions!
I would imagine this (setting
In that case, at least if the intent is similar to how I've typically seen |
Based on comments in the Crossplane Slack channel, I’m moving this back to draft mode until the new style (I.e., array vs single value) |
@yardbirdsax thanks a lot for the clarification, makes total sense, let's try it out. I will be happy to test it e2e when we get to the final implementation. |
@yardbirdsax #195 should implement this. I will close this PR, feel free to reopen if needed 👍 |
Description of your changes
This is a work in progress; I'm just opening this in draft mode to signal intent in case other folks were thinking of the same thing (and to get early feedback if anyone wants to 😄 ). Once I finish the implementation, I'll mark it ready for review.
This adds the new ManagementPolicy field to the Workflow resource by updating the
crossplane/crossplane-runtime
module to the latestmaster
commit. It also resolves a dependency conflict when the API types or other packages from this module are imported simultaneously as ones from other providers, such asupbound/provider-aws
since those use the newercrossplane-runtime
versions already. Related dependencies have also been updated. I also updated the version ofgolangci-lint
; the older version (1.50.0) has some known performance issues that this resolves (see the various comments on golangci/golangci-lint#3414).I have:
make reviewable
to ensure this PR is ready for review.How has this code been tested
I ran the existing unit tests which all passed. I may try and add some new ones as part of finishing the implementation. When running the end-to-end tests (
make e2e
) it fails with the erroruptest-v0.5.0: error: No manifest to test provided.
; I confirmed this is also true on themain
head, so it seems to be an existing problem.