improve pipeline support for function-cidr

[anessi]

Description of your changes

Motivation: the current code does not allow to use multiple function calls in a pipeline, because it only allows to read from observed.composite.resource properties. In pipelines we need to be able to read values from other fields and use them as input value (e.g. prefix).

This PR implements these main changes:

• add possibility to set fully qualified field references (e.g. desired.composite.resource., observed.composite.resource. or context)
• add consistent field support for cidrFunc (this is a breaking change, but used for consistency reasons: use cidrFuncField instead of cidrFunc now)
• add output field

Please see the added example pipeline for a concrete example how I use the function of the use-case.

Fixes #32

The updated function is available for testing as:

apiVersion: pkg.crossplane.io/v1beta1
kind: Function
metadata:
  name: function-cidr
spec:
  package: xpkg.upbound.io/anessi/function-cidr:v0.5.4

Please note that I'm not a GO developer, so review the code carefully as I might have missed some obvious things 😃

I have:

• Read and followed Upbound's contribution process.

[humoflife]

Do you need the backslashes in. apis/composition-pipeline-context.yaml here: prefixField: context.apiextensions\.crossplane\.io/extra-resources.XCluster.0.spec.cidrBlock ? If not, please remove them.

[humoflife]

When running make render, rendering cidrsubnetloop and cidrsubnets stopped working. The others are ok.

Please take a look.

Rendering examples/xr-cidrsubnetloop.yaml...
crossplane: error: cannot render composite resource: pipeline step "cidr" returned a fatal result: cannot get output from field spec.parameters.output for XCIDR: spec.parameters.output: no such field
Rendering examples/xr-cidrsubnets.yaml...
crossplane: error: cannot render composite resource: pipeline step "cidr" returned a fatal result: cannot get output from field spec.parameters.output for XCIDR: spec.parameters.output: no such field
make: *** [render] Error 1

[humoflife]

function-cidr is offering cidrFunc and cidrFuncField inputs.

• cidrFunc allows direct specification of the desired CIDR function.
• cidrFuncField allows specification of the field where the CIDR function name is specified, e.g. cidrhost.

The description of the changes includes:

... add consistent field support for cidrFunc (this is a breaking change, but used for consistency reasons: use cidrFuncField instead of cidrFunc now)

What is the breaking change and why should not both cidrFunc and cidrFuncField be supported with their original intent and in analogy to other parameters?

[humoflife]

Please add test coverage in fn_test.go.

[humoflife]

Was the output field not supported prior to this PR? What was added?

[humoflife]

Please run all yaml files through yamlfmt (https://formulae.brew.sh/formula/yamlfmt if you are using Mac OSX) to update the indentation of arrays or manually update, e.g.

foo:
  - bar1
  - bar2

versus:

foo:
- bar1
- bae2

[humoflife]

In the comment in input/v1beta1/parameters.go, please replace

// cidrfunc is one of cidrhost, cidrnetmast, cidesubnet, cidrsubnets, cidrsubnetloop

with

// cidrfunc is one of cidrhost, cidrnetmast, cidrsubnet, cidrsubnets, cidrsubnetloop

[anessi]

Do you need the backslashes in. apis/composition-pipeline-context.yaml here: prefixField: context.apiextensions\.crossplane\.io/extra-resources.XCluster.0.spec.cidrBlock ? If not, please remove them.

They are needed because it's a key with dots in the name (apiextensions.crossplane.io/extra-resources). See https://github.com/crossplane-contrib/function-extra-resources/tree/main

make render

This is fixed now. Thanks for noticing.

cidrFuncField vs cidrFunc

There was no cidrFuncField before, but instead cidrFunc had the same behavior as now cidrFuncField. This was inconsistent and I fixed this to make it consistent. See https://github.com/upbound/function-cidr/pull/33/files#diff-e7b8a7f13b611cc9ad80aad4c3c35c12930ad31ddfb43b721a692c0c8d069391

outputField vs output

output was not supported, but only outputField

yamlfmt

The files are formatted with yamlfmt now.

// cidrfunc is one of cidrhost, cidrnetmast, cidrsubnet, cidrsubnets, cidrsubnetloop

Fixed the comments. There was a second spelling mistake (cidrnetmast).

Please add test coverage in fn_test.go.

Looks like the project does not have any tests so far. What would you like me to do?

Wouldn't it be easier if you add the comments directly on the code instead of different comments here? Makes it a bit hard to give replies to individual comments.

[haarchri]

@anessi sorry for late Response - can you rebase your PR?

[haarchri]

and please Check we merged a few PRs recently

[anessi]

@haarchri : I have update the code accordingly. It got a bit simpler due to the improvements implemented in 0800924 (which is covering some of my previous changes).

I also added 2 tests.