Skip to content

fix: Local_resource and kubectl apply could risk deploying to the wrong context#66

Merged
danielpanzella merged 3 commits intomainfrom
danielpanzella/fix-tilt-cross-context-issue
Mar 17, 2025
Merged

fix: Local_resource and kubectl apply could risk deploying to the wrong context#66
danielpanzella merged 3 commits intomainfrom
danielpanzella/fix-tilt-cross-context-issue

Conversation

@danielpanzella
Copy link
Copy Markdown
Contributor

@danielpanzella danielpanzella commented Feb 25, 2025

Summary by CodeRabbit

  • Refactor
    • Streamlined deployment configurations for improved resource management and clearer integration definitions.
  • Chores
    • Updated security settings to align with current deployment practices and enhance overall system consistency.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Feb 25, 2025

Warning

Rate limit exceeded

@danielpanzella has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 24 minutes and 1 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between f37e4b6 and d26d398.

📒 Files selected for processing (1)
  • Tiltfile (3 hunks)

Walkthrough

The pull request simplifies resource management in the Tiltfile and streamlines security settings in the manager deployment. The yaml() function is removed and replaced by direct k8s_yaml and k8s_resource calls for Minio, WandB, CRD, and RBAC resources. Additionally, the runAsNonRoot: true directive is removed from the manager's security context, along with related comments. These changes reorganize how configurations are applied and managed in the Kubernetes environment.

Changes

File(s) Change Summary
Tiltfile Removed the yaml() function; replaced local_resource calls with k8s_yaml/k8s_resource for Minio, WandB, CRD, and RBAC resource definitions.
config/manager/manager.yaml Removed runAsNonRoot: true from the securityContext and deleted associated comments regarding container non-root enforcement.

Sequence Diagram(s)

sequenceDiagram
    participant T as Tiltfile
    participant KR as k8s_yaml/k8s_resource
    participant K8s as Kubernetes Cluster

    T->>KR: Define resources (Minio, WandB, CRD, RBAC)
    KR->>K8s: Apply resource configurations
    K8s-->>KR: Return deployment status
    KR-->>T: Propagate status update
Loading

Possibly related PRs

Suggested reviewers

  • zacharyblasczyk

Poem

I'm a happy rabbit, hopping with glee,
No more yaml magic—just clarity to see.
With k8s_yaml and k8s_resource in the mix,
My code hops along, neat and slick.
Carrots and code, in a dance so free!
🥕🐇


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
Tiltfile (1)

108-109: Commented code should be removed

These commented-out lines are now redundant since you've implemented the proper Tilt-native approach in lines 99-106.

-    #local_resource('Sample YAML', 'kubectl apply -f ./hack/testing-manifests/wandb/' + settings.get('wandbCRD') +
-    #               '.yaml', deps=["./hack/testing-manifests/wandb/" + settings.get('wandbCRD') + ".yaml"], resource_deps=["controller-manager"])
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4249ad7 and f37e4b6.

📒 Files selected for processing (2)
  • Tiltfile (3 hunks)
  • config/manager/manager.yaml (0 hunks)
💤 Files with no reviewable changes (1)
  • config/manager/manager.yaml
⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: Test
🔇 Additional comments (4)
Tiltfile (4)

67-75: Improved security by replacing kubectl direct calls with Tilt's k8s functions

This change replaces the previous local_resource with kubectl apply command with Tilt's native k8s_yaml and k8s_resource functions. This is a significant security improvement as it ensures Minio is only deployed to contexts explicitly allowed by allow_k8s_contexts, preventing accidental deployments to unintended or production environments.


77-77: Enhanced safety with Tilt's k8s_yaml for CRD deployment

Converting the direct kubectl command to Tilt's native function prevents deployment to unintended contexts. This approach also captures the output of the local command, making it more consistent with Tilt's declarative approach.


79-90: Improved resource organization with explicit object definitions

This change properly organizes CRD and RBAC resources with explicit object type definitions, making the deployment more structured and transparent. The explicit naming of cluster-scoped resources (clusterrole, clusterrolebinding) is particularly important for security awareness.


99-106: Enhanced safety for Wandb resource deployment

Similar to other changes, this replaces a direct kubectl command with Tilt's native functions, ensuring deployments only target allowed contexts. The explicit dependency on "operator-controller-manager" maintains the correct deployment order.

Copy link
Copy Markdown
Contributor

@zacharyblasczyk zacharyblasczyk left a comment

Choose a reason for hiding this comment

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

🚢

@danielpanzella danielpanzella merged commit a90a270 into main Mar 17, 2025
@danielpanzella danielpanzella deleted the danielpanzella/fix-tilt-cross-context-issue branch March 17, 2025 22:29
jsbroks pushed a commit that referenced this pull request Mar 17, 2025
### [1.19.2](v1.19.1...v1.19.2) (2025-03-17)

### Bug Fixes

* Local_resource and kubectl apply could risk deploying to the wrong context ([#66](#66)) ([a90a270](a90a270))
@jsbroks
Copy link
Copy Markdown
Member

jsbroks commented Mar 17, 2025

This PR is included in version 1.19.2 🎉

sqaisar pushed a commit to sqaisar/wandb-operator that referenced this pull request Apr 11, 2025
## 1.0.0 (2025-04-11)

### Features

* Add active-state cm ([wandb#2](https://github.com/sqaisar/wandb-operator/issues/2)) ([5a6c4c3](5a6c4c3))
* Add caching for deployer release requests ([1185b40](1185b40))
* Add events recording ([388d37b](388d37b))
* Add helm support ([077765c](077765c))
* Add option to set reconcileFrequency ([484c014](484c014))
* Add support for helm repo releases ([dfef752](dfef752))
* Add support for release from a git repository ([8a6b073](8a6b073))
* Adding owner ref and wait and timeout  to uninstall  ([wandb#51](https://github.com/sqaisar/wandb-operator/issues/51)) ([f21fd6d](f21fd6d))
* Allow the operator to support installation without cluster level permissions ([wandb#16](https://github.com/sqaisar/wandb-operator/issues/16)) ([6f29a3e](6f29a3e))
* Make wandb operator available on OperatorHub ([wandb#32](https://github.com/sqaisar/wandb-operator/issues/32)) ([1a59dab](1a59dab))
* **operator:** Add airgapped support ([wandb#12](https://github.com/sqaisar/wandb-operator/issues/12)) ([bfd3796](bfd3796))
* Prevent Logging of Sensitive info in Plain Text ([wandb#31](https://github.com/sqaisar/wandb-operator/issues/31)) ([5530cb3](5530cb3))
* Prevent Logging of Sensitive info in Plain Text ([wandb#35](https://github.com/sqaisar/wandb-operator/issues/35)) ([9a752fd](9a752fd))
* Release Version Pinning Init ([wandb#28](https://github.com/sqaisar/wandb-operator/issues/28)) ([dfe8bda](dfe8bda))
* Replace base image with RHEL UBI ([wandb#44](https://github.com/sqaisar/wandb-operator/issues/44)) ([12497d2](12497d2))
* Support for deploymenting via jobs ([da801ea](da801ea))
* Updated license.go file to include the feature for licenseSecret as well ([wandb#54](https://github.com/sqaisar/wandb-operator/issues/54)) ([bb55caa](bb55caa))
* Use container based deployments only ([3e6b222](3e6b222))
* use secrets instead of configmaps ([049797f](049797f))

### Bug Fixes

* add applied config to download bundle ([bef77c2](bef77c2))
* Add console namespace and service name to config properties ([0b9efef](0b9efef))
* Add debugging for installing release ([893ebd9](893ebd9))
* add gh token for ci ([72d456f](72d456f))
* add license log ([wandb#11](https://github.com/sqaisar/wandb-operator/issues/11)) ([e129fab](e129fab))
* Add operator namespace env ([846731a](846731a))
* add operator properties to config ([b5f48f0](b5f48f0))
* add pnpm, node and git to docker image ([176b6f0](176b6f0))
* Add Tilt configs for local development ([wandb#53](https://github.com/sqaisar/wandb-operator/issues/53)) ([5ef82b5](5ef82b5))
* added changelog commits ([61b5f5d](61b5f5d))
* Assign metadata instead of merging it ([908c839](908c839))
* Basic Auth Fix ([wandb#56](https://github.com/sqaisar/wandb-operator/issues/56)) ([414b2cf](414b2cf))
* Bump controller tools version to latest ([wandb#13](https://github.com/sqaisar/wandb-operator/issues/13)) ([c52dbb6](c52dbb6))
* Bump deps ([wandb#36](https://github.com/sqaisar/wandb-operator/issues/36)) ([eefb59c](eefb59c))
* Bump deps ([wandb#70](https://github.com/sqaisar/wandb-operator/issues/70)) ([11ba9f8](11ba9f8))
* Channel spec not getting applied correctly ([6e763a8](6e763a8))
* Charts download ([57355ce](57355ce))
* Clean up docker image ([ef7c629](ef7c629))
* clean up env for image push ([7213ed2](7213ed2))
* Correct merge order ([cd49cef](cd49cef))
* correctly check if chart is installed based on status ([384d330](384d330))
* Create release rc files ([f7f4622](f7f4622))
* Debug logging errors ([wandb#26](https://github.com/sqaisar/wandb-operator/issues/26)) ([a641621](a641621))
* Debug logging the cache ([wandb#21](https://github.com/sqaisar/wandb-operator/issues/21)) ([26e8fd5](26e8fd5))
* Debugging logic ([wandb#22](https://github.com/sqaisar/wandb-operator/issues/22)) ([2c019b8](2c019b8))
* Default to dev mode ([d961f77](d961f77))
* docker build ([d160a9c](d160a9c))
* docker image push ([e08b3da](e08b3da))
* Git release pulls correctly ([d47aebd](d47aebd))
* init controller ([0f0a9e9](0f0a9e9))
* install go version ([6664b4b](6664b4b))
* Install kubectl in docker image ([e5df9de](e5df9de))
* Jobs work? ([9972d26](9972d26))
* kubectl not working in docker image ([ffc694e](ffc694e))
* Local_resource and kubectl apply could risk deploying to the wrong context ([wandb#66](https://github.com/sqaisar/wandb-operator/issues/66)) ([a90a270](a90a270))
* lock pnpm version ([c2608f7](c2608f7))
* Log the diff of specs ([wandb#23](https://github.com/sqaisar/wandb-operator/issues/23)) ([c0ea0d8](c0ea0d8))
* Look for secret in namespace of wandb CR ([wandb#78](https://github.com/sqaisar/wandb-operator/issues/78)) ([e374c9a](e374c9a))
* Mask sensitive values in log ([wandb#14](https://github.com/sqaisar/wandb-operator/issues/14)) ([514336d](514336d))
* merge func ([94aa0d0](94aa0d0))
* Output json format logs ([90af7b6](90af7b6))
* Pass namespace into chart ([e8e0b8f](e8e0b8f))
* pass spec namespace and name ([79d77f2](79d77f2))
* Preserve unknown fields ([565a25f](565a25f))
* properly get license ([6ff6533](6ff6533))
* Properly merge chart specs together ([37c41bc](37c41bc))
* Properly parse chart from deployer ([5eabdfe](5eabdfe))
* Properly set namespace for deployments ([53f51a9](53f51a9))
* Properly update complete status ([86a5196](86a5196))
* push images to dockerhub ([d4cdd27](d4cdd27))
* refactor spec ([87be86b](87be86b))
* Refactor specs ([7c6da34](7c6da34))
* Release needs ginkgo ([wandb#65](https://github.com/sqaisar/wandb-operator/issues/65)) ([c51df78](c51df78))
* remove console ([fba45ee](fba45ee))
* remove debugging logs ([d4da31f](d4da31f))
* remove submodule ([bdb408a](bdb408a))
* Remove ui building step ([08ee985](08ee985))
* Rename config -> values and release -> chart ([519cd1b](519cd1b))
* Rename config spec cfs ([672100a](672100a))
* rename configs ([8727281](8727281))
* rename docker variables ([274e20c](274e20c))
* rename versioning step name ([77bf4ed](77bf4ed))
* reorder backup ([ab66486](ab66486))
* revert to v2 for semver ([535a721](535a721))
* Save active spec metadata ([47bd862](47bd862))
* Secret reading metadata ([6dab7ed](6dab7ed))
* secrets stored with correct values ([f6d61e9](f6d61e9))
* Serve console with gin ([c9e04aa](c9e04aa))
* set namespace when running kubectl apply ([1d6f00c](1d6f00c))
* Setting cached release namespace incorrectly ([e585555](e585555))
* Simplify docker image ([1cf55e4](1cf55e4))
* Support Openshift permissions schema for the helm cache ([wandb#17](https://github.com/sqaisar/wandb-operator/issues/17)) ([b498f79](b498f79))
* TLS ([wandb#67](https://github.com/sqaisar/wandb-operator/issues/67)) ([0d3013c](0d3013c))
* Tmp directory permissions ([b0820f5](b0820f5))
* Update to fix some CVEs ([wandb#55](https://github.com/sqaisar/wandb-operator/issues/55)) ([9a34cbe](9a34cbe))
* upgrade semantic to v3 ([594c463](594c463))
* Use cdk8s image for apply container ([189bc08](189bc08))
* Use deployer release channels ([480b380](480b380))
* Using validate for job spec ([5c7ff66](5c7ff66))
* x-kubernetes-preserve-unknown-fields ([bedac52](bedac52))
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