implement helper angryjet functions by ldalorion · Pull Request #86 · crossplane/crossplane-tools

ldalorion · 2025-03-13T20:19:10Z

Description of your changes

The removal of the pointer helper methods in crossplane-runtime v1.19.0 will now make angryjet generate invalid code.
https://github.com/crossplane/crossplane-runtime/pull/780/files

Fixes # crossplane/crossplane-runtime#762

I have:

Read and followed Crossplane's contribution process.
Run make reviewable test to ensure this PR is ready for review.

How has this code been tested

Updated go.mod file of provider-upbound to point to my changes locally

replace github.com/crossplane/crossplane-tools => /Users/dajohnson/projects/crossplane-tools

ran the go commands to pull the changes into the environment

go mod tidy
go mod vendor
make build
make reviewable test

deployed the changes to my local kind cluster

make dev

applied resources to the cluster and verified that they eventually were synced and ready.

(base) {25-03-15 21:24}~/20250314-provider-upbound-integration-test ➭ k get managed
NAME                                   READY   SYNCED   EXTERNAL-NAME                          AGE
robot.iam.upbound.io/dalorions-robot   True    True     419bb021-83e2-4783-928a-ad6847e90260   102s

NAME                                READY   SYNCED   EXTERNAL-NAME                          AGE
team.iam.upbound.io/dalorion-team   True    True     f726ad76-adf6-45d1-a6b2-f68f7b945d17   102s

NAME                                         READY   SYNCED   EXTERNAL-NAME                          AGE
token.iam.upbound.io/dalorions-robot-token   True    True     79e2daa0-1e42-4c5f-8cbb-5d0b98bfc8ef   102s

NAME                                         READY   SYNCED   EXTERNAL-NAME   AGE
permission.repository.upbound.io/alper-dev   True    True                     102s

NAME                                                READY   SYNCED   EXTERNAL-NAME      AGE
repository.repository.upbound.io/provider-upbound   True    True     provider-upbound   102s

~~TODO: apply changes to local provider-upjet-gcp and run uptest locally~~

Ran uptest against provider-upjet-gcp with it pointing to my changes. Did the same setup for provider-upbound but ran make uptest against the storage bucket example.

export UPTEST_EXAMPLE_LIST="examples/storage/v1beta2/bucket.yaml"

test passed

 logger.go:42: 17:48:55 | case/2-import | running command: [sh -c echo "Dump MR manifests for the import assertion step:"; ${KUBECTL} get managed -o yaml]
    logger.go:42: 17:48:55 | case/2-import | Dump MR manifests for the import assertion step:
    logger.go:42: 17:49:00 | case/2-import | apiVersion: v1
    logger.go:42: 17:49:00 | case/2-import | items:
    logger.go:42: 17:49:00 | case/2-import | - apiVersion: storage.gcp.upbound.io/v1beta2
    logger.go:42: 17:49:00 | case/2-import |   kind: Bucket
    logger.go:42: 17:49:00 | case/2-import |   metadata:
    logger.go:42: 17:49:00 | case/2-import |     annotations:
    logger.go:42: 17:49:00 | case/2-import |       crossplane.io/external-create-failed: "2025-03-20T22:48:05Z"
    logger.go:42: 17:49:00 | case/2-import |       crossplane.io/external-create-pending: "2025-03-20T22:48:05Z"
    logger.go:42: 17:49:00 | case/2-import |       crossplane.io/external-create-succeeded: "2025-03-20T22:44:04Z"
    logger.go:42: 17:49:00 | case/2-import |       crossplane.io/external-name: bucket-op-u4keh593
    logger.go:42: 17:49:00 | case/2-import |       crossplane.io/paused: "false"
    logger.go:42: 17:49:00 | case/2-import |       meta.upbound.io/example-id: storage/v1beta2/bucket
    logger.go:42: 17:49:00 | case/2-import |       upjet.upbound.io/test: "true"
    logger.go:42: 17:49:00 | case/2-import |       uptest-old-id: bucket-op-u4keh593
    logger.go:42: 17:49:00 | case/2-import |     creationTimestamp: "2025-03-20T22:42:03Z"
    logger.go:42: 17:49:00 | case/2-import |     finalizers:
    logger.go:42: 17:49:00 | case/2-import |     - finalizer.managedresource.crossplane.io
    logger.go:42: 17:49:00 | case/2-import |     generation: 2
    logger.go:42: 17:49:00 | case/2-import |     labels:
    logger.go:42: 17:49:00 | case/2-import |       testing.upbound.io/example-name: bucket
    logger.go:42: 17:49:00 | case/2-import |     name: bucket-op-u4keh593
    logger.go:42: 17:49:00 | case/2-import |     resourceVersion: "6180"
    logger.go:42: 17:49:00 | case/2-import |     uid: c95290df-d4bf-43ab-8b90-dd426bf39271
    logger.go:42: 17:49:00 | case/2-import |   spec:
    logger.go:42: 17:49:00 | case/2-import |     deletionPolicy: Delete
    logger.go:42: 17:49:00 | case/2-import |     forProvider:
    logger.go:42: 17:49:00 | case/2-import |       cors:
    logger.go:42: 17:49:00 | case/2-import |       - maxAgeSeconds: 3600
    logger.go:42: 17:49:00 | case/2-import |         method:
    logger.go:42: 17:49:00 | case/2-import |         - GET
    logger.go:42: 17:49:00 | case/2-import |         - HEAD
    logger.go:42: 17:49:00 | case/2-import |         - PUT
    logger.go:42: 17:49:00 | case/2-import |         - POST
    logger.go:42: 17:49:00 | case/2-import |         - DELETE
    logger.go:42: 17:49:00 | case/2-import |         origin:
    logger.go:42: 17:49:00 | case/2-import |         - http://image-store.com
    logger.go:42: 17:49:00 | case/2-import |         responseHeader:
    logger.go:42: 17:49:00 | case/2-import |         - '*'
    logger.go:42: 17:49:00 | case/2-import |       forceDestroy: true
    logger.go:42: 17:49:00 | case/2-import |       location: US
    logger.go:42: 17:49:00 | case/2-import |       project: official-provider-testing
    logger.go:42: 17:49:00 | case/2-import |       publicAccessPrevention: inherited
    logger.go:42: 17:49:00 | case/2-import |       rpo: DEFAULT
    logger.go:42: 17:49:00 | case/2-import |       softDeletePolicy:
    logger.go:42: 17:49:00 | case/2-import |         retentionDurationSeconds: 604800
    logger.go:42: 17:49:00 | case/2-import |       storageClass: STANDARD
    logger.go:42: 17:49:00 | case/2-import |       uniformBucketLevelAccess: true
    logger.go:42: 17:49:00 | case/2-import |       website:
    logger.go:42: 17:49:00 | case/2-import |         mainPageSuffix: index.html
    logger.go:42: 17:49:00 | case/2-import |         notFoundPage: 404.html
    logger.go:42: 17:49:00 | case/2-import |     initProvider: {}
    logger.go:42: 17:49:00 | case/2-import |     managementPolicies:
    logger.go:42: 17:49:00 | case/2-import |     - '*'
    logger.go:42: 17:49:00 | case/2-import |     providerConfigRef:
    logger.go:42: 17:49:00 | case/2-import |       name: default
    logger.go:42: 17:49:00 | case/2-import |   status:
    logger.go:42: 17:49:00 | case/2-import |     atProvider:
    logger.go:42: 17:49:00 | case/2-import |       cors:
    logger.go:42: 17:49:00 | case/2-import |       - maxAgeSeconds: 3600
    logger.go:42: 17:49:00 | case/2-import |         method:
    logger.go:42: 17:49:00 | case/2-import |         - GET
    logger.go:42: 17:49:00 | case/2-import |         - HEAD
    logger.go:42: 17:49:00 | case/2-import |         - PUT
    logger.go:42: 17:49:00 | case/2-import |         - POST
    logger.go:42: 17:49:00 | case/2-import |         - DELETE
    logger.go:42: 17:49:00 | case/2-import |         origin:
    logger.go:42: 17:49:00 | case/2-import |         - http://image-store.com
    logger.go:42: 17:49:00 | case/2-import |         responseHeader:
    logger.go:42: 17:49:00 | case/2-import |         - '*'
    logger.go:42: 17:49:00 | case/2-import |       defaultEventBasedHold: false
    logger.go:42: 17:49:00 | case/2-import |       enableObjectRetention: false
    logger.go:42: 17:49:00 | case/2-import |       forceDestroy: true
    logger.go:42: 17:49:00 | case/2-import |       id: bucket-op-u4keh593
    logger.go:42: 17:49:00 | case/2-import |       location: US
    logger.go:42: 17:49:00 | case/2-import |       project: official-provider-testing
    logger.go:42: 17:49:00 | case/2-import |       projectNumber: 990150596479
    logger.go:42: 17:49:00 | case/2-import |       publicAccessPrevention: inherited
    logger.go:42: 17:49:00 | case/2-import |       requesterPays: false
    logger.go:42: 17:49:00 | case/2-import |       rpo: DEFAULT
    logger.go:42: 17:49:00 | case/2-import |       selfLink: https://www.googleapis.com/storage/v1/b/bucket-op-u4keh593
    logger.go:42: 17:49:00 | case/2-import |       softDeletePolicy:
    logger.go:42: 17:49:00 | case/2-import |         effectiveTime: "2025-03-20T22:48:06.850Z"
    logger.go:42: 17:49:00 | case/2-import |         retentionDurationSeconds: 604800
    logger.go:42: 17:49:00 | case/2-import |       storageClass: STANDARD
    logger.go:42: 17:49:00 | case/2-import |       uniformBucketLevelAccess: true
    logger.go:42: 17:49:00 | case/2-import |       url: gs://bucket-op-u4keh593
    logger.go:42: 17:49:00 | case/2-import |       website:
    logger.go:42: 17:49:00 | case/2-import |         mainPageSuffix: index.html
    logger.go:42: 17:49:00 | case/2-import |         notFoundPage: 404.html
    logger.go:42: 17:49:00 | case/2-import |     conditions:
    logger.go:42: 17:49:00 | case/2-import |     - lastTransitionTime: "2025-03-20T22:48:49Z"
    logger.go:42: 17:49:00 | case/2-import |       reason: ReconcileSuccess
    logger.go:42: 17:49:00 | case/2-import |       status: "True"
    logger.go:42: 17:49:00 | case/2-import |       type: Synced
    logger.go:42: 17:49:00 | case/2-import |     - lastTransitionTime: "2025-03-20T22:48:49Z"
    logger.go:42: 17:49:00 | case/2-import |       reason: Available
    logger.go:42: 17:49:00 | case/2-import |       status: "True"
    logger.go:42: 17:49:00 | case/2-import |       type: Ready
    logger.go:42: 17:49:00 | case/2-import |     - lastTransitionTime: "2025-03-20T22:48:49Z"
    logger.go:42: 17:49:00 | case/2-import |       reason: UpToDate
    logger.go:42: 17:49:00 | case/2-import |       status: "True"
    logger.go:42: 17:49:00 | case/2-import |       type: Test
    logger.go:42: 17:49:00 | case/2-import |     - lastTransitionTime: "2025-03-20T22:48:49Z"
    logger.go:42: 17:49:00 | case/2-import |       reason: Success
    logger.go:42: 17:49:00 | case/2-import |       status: "True"
    logger.go:42: 17:49:00 | case/2-import |       type: LastAsyncOperation
    logger.go:42: 17:49:00 | case/2-import | kind: List
    logger.go:42: 17:49:00 | case/2-import | metadata:
    logger.go:42: 17:49:00 | case/2-import |   resourceVersion: ""
    logger.go:42: 17:49:00 | case/2-import | running command: [/Users/dajohnson/projects/provider-upjet-gcp/.cache/tools/darwin_arm64/kubectl-v1.24.3 wait bucket.storage.gcp.upbound.io/bucket-op-u4keh593 --for=condition=Test --timeout 10s]
    logger.go:42: 17:49:02 | case/2-import | bucket.storage.gcp.upbound.io/bucket-op-u4keh593 condition met
    logger.go:42: 17:49:02 | case/2-import | running command: [sh -c new_id="$(${KUBECTL} get bucket.storage.gcp.upbound.io/bucket-op-u4keh593 -o=jsonpath='{.status.atProvider.id}')" && old_id="$(${KUBECTL} get bucket.storage.gcp.upbound.io/bucket-op-u4keh593 -o=jsonpath='{.metadata.annotations.uptest-old-id}')" && [ "$new_id" = "$old_id" ]]
    logger.go:42: 17:49:06 | case/2-import | test step completed 2-import
    logger.go:42: 17:49:06 | case/3-delete | starting test step 3-delete
    logger.go:42: 17:49:06 | case/3-delete | running command: [/Users/dajohnson/projects/provider-upjet-gcp/.cache/tools/darwin_arm64/kubectl-v1.24.3 delete bucket.storage.gcp.upbound.io/bucket-op-u4keh593 --wait=false --ignore-not-found]
    logger.go:42: 17:49:09 | case/3-delete | bucket.storage.gcp.upbound.io "bucket-op-u4keh593" deleted
I0320 17:49:10.250890   96041 request.go:655] Throttling request took 1.045592792s, request: GET:https://127.0.0.1:52511/apis/pkg.crossplane.io/v1?timeout=32s
    logger.go:42: 17:49:16 | case/3-delete | running command: [sh -c echo "Dump MR manifests for the delete assertion step:"; ${KUBECTL} get managed -o yaml]
    logger.go:42: 17:49:16 | case/3-delete | Dump MR manifests for the delete assertion step:
    logger.go:42: 17:49:20 | case/3-delete | apiVersion: v1
    logger.go:42: 17:49:20 | case/3-delete | items: []
    logger.go:42: 17:49:20 | case/3-delete | kind: List
    logger.go:42: 17:49:20 | case/3-delete | metadata:
    logger.go:42: 17:49:20 | case/3-delete |   resourceVersion: ""
    logger.go:42: 17:49:20 | case/3-delete | running command: [sh -c echo "Dump Claim manifests for the delete assertion step:" || ${KUBECTL} get claim --all-namespaces -o yaml]
    logger.go:42: 17:49:20 | case/3-delete | Dump Claim manifests for the delete assertion step:
    logger.go:42: 17:49:20 | case/3-delete | running command: [/Users/dajohnson/projects/provider-upjet-gcp/.cache/tools/darwin_arm64/kubectl-v1.24.3 wait bucket.storage.gcp.upbound.io/bucket-op-u4keh593 --for=delete --timeout 10s]
    logger.go:42: 17:49:23 | case/3-delete | running command: [/Users/dajohnson/projects/provider-upjet-gcp/.cache/tools/darwin_arm64/kubectl-v1.24.3 wait managed --all --for=delete --timeout 10s]
    logger.go:42: 17:49:27 | case/3-delete | test step completed 3-delete
I0320 17:49:28.943696   96041 request.go:655] Throttling request took 1.044696708s, request: GET:https://127.0.0.1:52511/apis/bigquery.gcp.upbound.io/v1beta2?timeout=32s
    logger.go:42: 17:49:34 | case | Failed to collect events for case in ns kuttl-test-intimate-rooster: no matches for kind "Event" in version "events.k8s.io/v1beta1"
    logger.go:42: 17:49:34 | case | Trying with events eventsv1 API...
    logger.go:42: 17:49:34 | case | case events from ns kuttl-test-intimate-rooster:
    logger.go:42: 17:49:34 | case | Deleting namespace: kuttl-test-intimate-rooster
=== CONT  kuttl
    harness.go:402: run tests finished
    harness.go:511: cleaning up
    harness.go:553: skipping cluster tear down
    harness.go:554: to connect to the cluster, run: export KUBECONFIG="/Users/dajohnson/projects/provider-upjet-gcp/kubeconfig"
--- PASS: kuttl (486.62s)
    --- PASS: kuttl/harness (0.00s)
        --- PASS: kuttl/harness/case (465.58s)

negz · 2025-03-21T18:51:56Z

Thanks @ldalorion!

I know in a lot of places we've started using https://pkg.go.dev/k8s.io/utils/ptr instead of the pointer utils that used to be in crossplane-runtime. Do you think we could use that library here in crossplane-tools?

ldalorion · 2025-03-21T22:34:31Z

Yes! Started to make changes using that utility but replacing the functions that looped through each pointer and floating numbers was not as straightforward. So I just did a straight copypasta from crossplane-runtime first. I wanted to make sure that I was on the right path before investing anymore time and effort.

ldalorion · 2025-03-25T18:51:18Z

@negz Soooo i tried to use them as much as possible but since we're converting strings to numbers (floats/ints) and back there's not much that the utility can replace. 😬

negz · 2025-03-25T19:31:12Z

+	return strconv.FormatInt(*v, 10)
+}
+
+// ToPtrValue adapts a ResolvedValue for use as a string pointer field.


Suggested change

// ToPtrValue adapts a ResolvedValue for use as a string pointer field.

// ToPtrValue adapts a ResolvedValue for use as a string pointer field.

//

// Deprecated: Use ptr.To from k8s.io/utils/ptr.

negz · 2025-03-25T19:31:30Z

+// throughout the Crossplane codebase. We duplicate them here to reduce the
+// number of packages our API types have to import to support references.
+
+// FromPtrValue adapts a string pointer field for use as a CurrentValue.


// // Deprecated: Use ptr.Deref from k8s.io/utils/ptr.

Not sure what happened here - this was supposed to be a suggestion to expand the comment.

…ions Signed-off-by: L. Dalorion Johnson <ldalorion@users.noreply.github.com>

ldalorion · 2025-03-25T21:58:51Z

@negz Added an additional change to update the generated code to not use the now deprecated helper functions FromPtrValue and ToPtrValue. I reran my upjet test case pointing to my local angryjet changes and everything passed.

--- PASS: kuttl (687.59s)
    --- PASS: kuttl/harness (0.00s)
        --- PASS: kuttl/harness/case (665.81s)
PASS
16:54:39 [ OK ] running automated tests

negz

Thanks for updating the generated code!

Duologic · 2025-04-18T10:42:27Z

Is there anything blocking this?

ldalorion changed the title ~~migrate off of deprecated crossplane-runtime helper functions in generated~~ migrate angryjet off of deprecated crossplane-runtime helper functions Mar 14, 2025

ldalorion force-pushed the update-resolver-code-to-not-use-deprecated-runtime-function branch 3 times, most recently from ac58d4d to e70fbb0 Compare March 16, 2025 02:28

ldalorion changed the title ~~migrate angryjet off of deprecated crossplane-runtime helper functions~~ implement helper angryjet functions Mar 16, 2025

ldalorion force-pushed the update-resolver-code-to-not-use-deprecated-runtime-function branch from e70fbb0 to 1546903 Compare March 20, 2025 23:03

ldalorion marked this pull request as ready for review March 20, 2025 23:07

ldalorion force-pushed the update-resolver-code-to-not-use-deprecated-runtime-function branch from 740ebfe to cac74f8 Compare March 25, 2025 18:48

negz approved these changes Mar 25, 2025

View reviewed changes

add helper functions; add k8s.io/utils/ptr dependency to helper funct…

6507791

…ions Signed-off-by: L. Dalorion Johnson <ldalorion@users.noreply.github.com>

ldalorion force-pushed the update-resolver-code-to-not-use-deprecated-runtime-function branch from cac74f8 to 6507791 Compare March 25, 2025 21:04

negz approved these changes Mar 27, 2025

View reviewed changes

This was referenced Apr 18, 2025

chore(deps): update dependencies crossplane/upjet#488

Merged

chore(deps): update terraform-provider-grafana to v3.22.3 grafana/crossplane-provider-grafana#273

Merged

negz merged commit 4d69eca into crossplane:master Apr 21, 2025
2 of 7 checks passed

jbw976 mentioned this pull request Apr 22, 2025

Rename pkg/helpers to pkg/convert #91

Merged

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

implement helper angryjet functions#86

implement helper angryjet functions#86
negz merged 1 commit intocrossplane:masterfrom
ldalorion:update-resolver-code-to-not-use-deprecated-runtime-function

ldalorion commented Mar 13, 2025 •

edited

Loading

Uh oh!

negz commented Mar 21, 2025

Uh oh!

ldalorion commented Mar 21, 2025

Uh oh!

ldalorion commented Mar 25, 2025

Uh oh!

negz Mar 25, 2025

Uh oh!

negz Mar 25, 2025 •

edited

Loading

Uh oh!

ldalorion commented Mar 25, 2025

Uh oh!

negz left a comment

Uh oh!

Duologic commented Apr 18, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

ldalorion commented Mar 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description of your changes

How has this code been tested

Uh oh!

negz commented Mar 21, 2025

Uh oh!

ldalorion commented Mar 21, 2025

Uh oh!

ldalorion commented Mar 25, 2025

Uh oh!

negz Mar 25, 2025

Choose a reason for hiding this comment

Uh oh!

negz Mar 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ldalorion commented Mar 25, 2025

Uh oh!

negz left a comment

Choose a reason for hiding this comment

Uh oh!

Duologic commented Apr 18, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ldalorion commented Mar 13, 2025 •

edited

Loading

negz Mar 25, 2025 •

edited

Loading