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 create/update bug #493

Merged
merged 9 commits into from
Dec 17, 2021
Merged

Fix create/update bug #493

merged 9 commits into from
Dec 17, 2021

Conversation

martyspiewak
Copy link
Contributor

@martyspiewak martyspiewak commented Dec 16, 2021

Changes proposed by this PR

  • Workload/deliverable use Get instead of List (by labels) to find existing objects
  • Runnable uses List by carto.run/runnable-name label only
  • Developer service account now requires Get permission

closes #484

Release Notes

Developer service account now requires Get permission

PR Checklist

Note: Please do not remove items. Mark items as done [x] or use strikethrough if you believe they are not relevant

  • Linked to a relevant issue. Eg: Fixes #123 or Updates #123
  • Removed non-atomic or wip commits
  • Filled in the Release Note section above
  • Modified the docs to match changes

@netlify
Copy link

netlify bot commented Dec 16, 2021

✔️ Deploy Preview for elated-stonebraker-105904 ready!

🔨 Explore the source changes: f1d2311

🔍 Inspect the deploy log: https://app.netlify.com/sites/elated-stonebraker-105904/deploys/61bcb8b2a82b920008648202

😎 Browse the preview: https://deploy-preview-493--elated-stonebraker-105904.netlify.app

* List by GVK, namespace, and labels instead of
  passing object and listing by it's characteristics
@martyspiewak martyspiewak force-pushed the 484-create-instead-of-update branch from e385c64 to 8f21296 Compare December 16, 2021 16:36
Copy link
Contributor

@emmjohnson emmjohnson left a comment

Choose a reason for hiding this comment

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

Test locally. LGTM

@martyspiewak martyspiewak marked this pull request as draft December 16, 2021 17:02
emmjohnson and others added 8 commits December 16, 2021 17:03
Co-authored-by: Marty Spiewak <mspiewak@vmware.com>
Co-authored-by: Marty Spiewak <mspiewak@vmware.com>
Co-authored-by: Marty Spiewak <mspiewak@vmware.com>
Co-authored-by: Marty Spiewak <mspiewak@vmware.com>
Co-authored-by: Marty Spiewak <mspiewak@vmware.com>
# Conflicts:
#	pkg/realizer/deliverable/component_test.go
#	pkg/realizer/workload/component_test.go
#	pkg/repository/repository.go
#	pkg/repository/repository_test.go
#	pkg/repository/repositoryfakes/fake_repository.go
Co-authored-by: Marty Spiewak <mspiewak@vmware.com>
@emmjohnson emmjohnson marked this pull request as ready for review December 17, 2021 16:20
@emmjohnson emmjohnson changed the title Filter ListUnstructured on carto labels only Fix create/update bug Dec 17, 2021
@martyspiewak martyspiewak merged commit ec82275 into main Dec 17, 2021
@martyspiewak martyspiewak deleted the 484-create-instead-of-update branch December 17, 2021 16:31
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.

bug: cartographer tries to create when it should update
4 participants