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] Support for resource ordering (implicit and explicit) #2894

Merged
merged 8 commits into from
Mar 21, 2024

Conversation

EronWright
Copy link
Contributor

@EronWright EronWright commented Mar 19, 2024

Proposed changes

This PR adds support for resource ordering within a ConfigGroup or ConfigFile. Two approaches are supported (and work in combination):

  1. implicit dependencies: the provider uses heuristics to install CRDs and namespaces first.
  2. explicit dependencies: the provider understands the config.kubernetes.io/depends-on annotation to explicitly declare a dependency on a given resource.

The implementation is based on kubernetes-sigs/cli-utils and its support for resource ordering (documentation).

To be clear, ordering across ConfigGroup resources is supported already, simply using the dependsOn option. This PR adds a more granular ordering within the group.

Testing

New test cases were added to verify the new behavior. Many tests rely on a common manifest file, and it is updated to include a CRD and a namespace.

Manual testing was performed with the following well-known manifests:

  1. knative-serving-core (ref)
  2. cert-manager (ref)

Example

Here's an example that installs cert-manager then provisions a certificate issuer and a TLS certificate. An explicit dependency is drawn between the Certificate and Issuer.

name: issue-2881-cert-manager
runtime: yaml
description: Installs cert-manager.  See https://cert-manager.io/docs/installation/kubectl/ for details.
variables: {}
resources:
  install:
    type: kubernetes:yaml/v2:ConfigGroup
    properties:
      files:
      - https://github.com/cert-manager/cert-manager/releases/download/v1.14.4/cert-manager.yaml
  test:
    type: kubernetes:yaml/v2:ConfigGroup
    options:
      dependsOn:
      - ${install}
    properties:
      yaml: |
        apiVersion: v1
        kind: Namespace
        metadata:
          name: cert-manager-test
        ---
        apiVersion: cert-manager.io/v1
        kind: Issuer
        metadata:
          name: test-selfsigned
          namespace: cert-manager-test
        spec:
          selfSigned: {}
        ---
        apiVersion: cert-manager.io/v1
        kind: Certificate
        metadata:
          name: selfsigned-cert
          namespace: cert-manager-test
          annotations:
            config.kubernetes.io/depends-on: cert-manager.io/namespaces/cert-manager-test/Issuer/test-selfsigned
        spec:
          dnsNames:
            - example.com
          secretName: selfsigned-cert-tls
          issuerRef:
            name: test-selfsigned

Within the stack state, one sees dependencies, e.g. on the Certificate resource.

{
    "urn": "urn:pulumi:dev::issue-2881-cert-manager::kubernetes:yaml/v2:ConfigGroup$kubernetes:cert-manager.io/v1:Certificate::test-cert-manager-test/selfsigned-cert",
    "id": "cert-manager-test/selfsigned-cert",
    "type": "kubernetes:cert-manager.io/v1:Certificate",
    "dependencies": [
        "urn:pulumi:dev::issue-2881-cert-manager::kubernetes:yaml/v2:ConfigGroup$kubernetes:cert-manager.io/v1:Issuer::test-cert-manager-test/test-selfsigned",
        "urn:pulumi:dev::issue-2881-cert-manager::kubernetes:yaml/v2:ConfigGroup$kubernetes:core/v1:Namespace::test-cert-manager-test"
    ]
}

Related issues (optional)

Closes #2881

@EronWright EronWright changed the base branch from master to eronwright/yaml-refactor March 19, 2024 21:53
Base automatically changed from eronwright/yaml-refactor to master March 19, 2024 21:56
@EronWright EronWright self-assigned this Mar 19, 2024
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 19, 2024

Codecov Report

Attention: Patch coverage is 81.13208% with 10 lines in your changes are missing coverage. Please review.

Project coverage is 27.19%. Comparing base (a6d364a) to head (119beeb).

Files Patch % Lines
provider/pkg/provider/yaml/v2/yaml.go 83.67% 4 Missing and 4 partials ⚠️
provider/pkg/provider/yaml/v2/configgroup.go 50.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2894      +/-   ##
==========================================
+ Coverage   26.97%   27.19%   +0.22%     
==========================================
  Files          53       53              
  Lines        7748     7780      +32     
==========================================
+ Hits         2090     2116      +26     
- Misses       5483     5486       +3     
- Partials      175      178       +3     

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

Copy link
Contributor

@blampe blampe left a comment

Choose a reason for hiding this comment

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

Looks reasonable - just some small comments/questions.

provider/pkg/gen/examples/overlays/configFileV2.md Outdated Show resolved Hide resolved
@@ -130,6 +137,41 @@ func yamlDecode(text string, _ *clients.DynamicClientSet) ([]unstructured.Unstru
return resources, nil
}

func Expand(objs []unstructured.Unstructured) ([]unstructured.Unstructured, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
func Expand(objs []unstructured.Unstructured) ([]unstructured.Unstructured, error) {
func expand(objs []unstructured.Unstructured) ([]unstructured.Unstructured, error) {

Or is there a reason this is public?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I made it public for symmetry with Parse and Register, all of which are useful when implementing a component resource. It so happens that ConfigGroup and ConfigFile are in the same package, but Chart wouldn't be. That said, I don't anticipate any actual usages.

Copy link
Contributor Author

@EronWright EronWright Mar 20, 2024

Choose a reason for hiding this comment

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

BTW, I plan to rename Expand to Normalize in this follow-up PR and perform a couple of other fixups.
#2897

provider/pkg/provider/yaml/v2/yaml.go Show resolved Hide resolved
provider/pkg/provider/yaml/v2/yaml.go Show resolved Hide resolved
@EronWright EronWright enabled auto-merge (squash) March 20, 2024 00:36
@lblackstone
Copy link
Member

This change also relates to the following issues:
#1098
#1671
#2227

Copy link
Member

@rquitales rquitales left a comment

Choose a reason for hiding this comment

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

LGTM

@EronWright EronWright merged commit e006410 into master Mar 21, 2024
18 checks passed
@EronWright EronWright deleted the eronwright/issue-2881 branch March 21, 2024 21:44
lumiere-bot bot referenced this pull request in coolguy1771/home-ops Apr 12, 2024
This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [@pulumi/kubernetes](https://pulumi.com)
([source](https://togithub.com/pulumi/pulumi-kubernetes)) | dependencies
| minor | [`4.9.1` ->
`4.10.0`](https://renovatebot.com/diffs/npm/@pulumi%2fkubernetes/4.9.1/4.10.0)
|

---

> [!WARNING]
> Some dependencies could not be looked up. Check the Dependency
Dashboard for more information.

---

### Release Notes

<details>
<summary>pulumi/pulumi-kubernetes (@&#8203;pulumi/kubernetes)</summary>

###
[`v4.10.0`](https://togithub.com/pulumi/pulumi-kubernetes/blob/HEAD/CHANGELOG.md#4100-April-11-2024)

[Compare
Source](https://togithub.com/pulumi/pulumi-kubernetes/compare/v4.9.1...v4.10.0)

- ConfigGroup V2
([https://github.com/pulumi/pulumi-kubernetes/pull/2844](https://togithub.com/pulumi/pulumi-kubernetes/pull/2844))
- ConfigFile V2
([https://github.com/pulumi/pulumi-kubernetes/pull/2862](https://togithub.com/pulumi/pulumi-kubernetes/pull/2862))
- Bugfix for ambiguous kinds
([https://github.com/pulumi/pulumi-kubernetes/pull/2889](https://togithub.com/pulumi/pulumi-kubernetes/pull/2889))
- \[yaml/v2] Support for resource ordering
[https://github.com/pulumi/pulumi-kubernetes/pull/2894](https://togithub.com/pulumi/pulumi-kubernetes/pull/2894)4)
- Bugfix for deployment await logic not referencing the correct
deployment status
([https://github.com/pulumi/pulumi-kubernetes/pull/2943](https://togithub.com/pulumi/pulumi-kubernetes/pull/2943))

##### New Features

A new MLC-based implementation of `ConfigGroup` and of `ConfigFile` is
now available in the "yaml/v2" package. These resources are
usable in all Pulumi languages, including Pulumi YAML and in the Java
Pulumi SDK.

Note that transformations aren't supported in this release (see
[https://github.com/pulumi/pulumi/issues/12996](https://togithub.com/pulumi/pulumi/issues/12996)).

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Renovate
Bot](https://togithub.com/renovatebot/renovate).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4yODcuMSIsInVwZGF0ZWRJblZlciI6IjM3LjI4Ny4xIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJyZW5vdmF0ZS9ucG0iLCJ0eXBlL21pbm9yIl19-->

Co-authored-by: lumiere-bot[bot] <98047013+lumiere-bot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Apply heuristics to apply CRDs and Namespaces before all other resources in components
4 participants