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

[yaml/v2] Improved resource prefix #2900

Merged
merged 3 commits into from
Mar 22, 2024

Conversation

EronWright
Copy link
Contributor

@EronWright EronWright commented Mar 21, 2024

Proposed changes

This PR changes the prefix character that is used, as per this discussion:
https://pulumi.slack.com/archives/C8R654Y6S/p1711035557620359

Tested with a few well-known manifests and seems to be a good improvement on readability.

     pulumi:pulumi:Stack                                                              issue-2881-cert-manager-dev                                            4 warn
     ├─ kubernetes:yaml/v2:ConfigGroup                                                install                                                                
 +   │  ├─ kubernetes:core/v1:Namespace                                               install:cert-manager                                        create     
 +   │  ├─ kubernetes:rbac.authorization.k8s.io/v1:ClusterRole                        install:cert-manager-cluster-view                           create     
 +   │  ├─ kubernetes:rbac.authorization.k8s.io/v1:ClusterRole                        install:cert-manager-controller-certificates                create     
 +   │  ├─ kubernetes:rbac.authorization.k8s.io/v1:ClusterRole                        install:cert-manager-edit                                   create     
 +   │  ├─ kubernetes:core/v1:ServiceAccount                                          install:cert-manager/cert-manager-webhook                   create     
 +   │  ├─ kubernetes:core/v1:ServiceAccount                                          install:cert-manager/cert-manager                           create     
 +   │  ├─ kubernetes:rbac.authorization.k8s.io/v1:ClusterRole                        install:cert-manager-view                                   create     
 +   │  ├─ kubernetes:rbac.authorization.k8s.io/v1:ClusterRole                        install:cert-manager-controller-issuers                     create     
 +   │  ├─ kubernetes:rbac.authorization.k8s.io/v1:ClusterRole                        install:cert-manager-controller-clusterissuers              create     
 +   │  ├─ kubernetes:core/v1:ServiceAccount                                          install:cert-manager/cert-manager-cainjector                create     
 +   │  ├─ kubernetes:rbac.authorization.k8s.io/v1:ClusterRole                        install:cert-manager-controller-ingress-shim                create     
 +   │  ├─ kubernetes:apiextensions.k8s.io/v1:CustomResourceDefinition                install:certificaterequests.cert-manager.io                 create     
 +   │  ├─ kubernetes:rbac.authorization.k8s.io/v1:ClusterRoleBinding                 install:cert-manager-controller-certificates                create     
 +   │  ├─ kubernetes:rbac.authorization.k8s.io/v1:ClusterRole                        install:cert-manager-cainjector                             create     
 +   │  ├─ kubernetes:core/v1:Service                                                 install:cert-manager/cert-manager                           create     
 +   │  ├─ kubernetes:rbac.authorization.k8s.io/v1:Role                               install:kube-system/cert-manager-cainjector:leaderelection  create     
 +   │  ├─ kubernetes:rbac.authorization.k8s.io/v1:RoleBinding                        install:cert-manager/cert-manager-webhook:dynamic-serving   create     
 +   │  ├─ kubernetes:rbac.authorization.k8s.io/v1:Role                               install:cert-manager/cert-manager-webhook:dynamic-serving   create     
 +   │  ├─ kubernetes:rbac.authorization.k8s.io/v1:ClusterRole                        install:cert-manager-controller-certificatesigningrequests  create     
 +   │  ├─ kubernetes:rbac.authorization.k8s.io/v1:ClusterRoleBinding                 install:cert-manager-controller-certificatesigningrequests  create     
 +   │  ├─ kubernetes:rbac.authorization.k8s.io/v1:Role                               install:kube-system/cert-manager:leaderelection             create     
 +   │  ├─ kubernetes:rbac.authorization.k8s.io/v1:ClusterRole                        install:cert-manager-controller-approve:cert-manager-io     create     

Related issues (optional)

Copy link

Does the PR have any schema changes?

Looking good! No breaking changes found.
No new resources/functions.

Copy link

codecov bot commented Mar 21, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 27.55%. Comparing base (5b47b32) to head (30cf1df).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #2900   +/-   ##
=======================================
  Coverage   27.55%   27.55%           
=======================================
  Files          53       53           
  Lines        7818     7818           
=======================================
  Hits         2154     2154           
  Misses       5485     5485           
  Partials      179      179           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@EronWright EronWright requested review from rquitales and a team March 21, 2024 19:06
CHANGELOG.md Outdated Show resolved Hide resolved
@EronWright EronWright force-pushed the eronwright/yamlv2-resource-prefix branch from 316ffc7 to 30cf1df Compare March 21, 2024 23:27
@EronWright EronWright added the impact/no-changelog-required This issue doesn't require a CHANGELOG update label Mar 21, 2024
@EronWright EronWright merged commit 99a295e into master Mar 22, 2024
18 checks passed
@EronWright EronWright deleted the eronwright/yamlv2-resource-prefix branch March 22, 2024 00:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
impact/no-changelog-required This issue doesn't require a CHANGELOG update
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants