fix: prevent dry-run from creating resources on consumer cluster

[olamilekan000]

Summary

Change fixes dry-run flag to ensure that binding assets are only saved locally without accessing 
the consumer cluster, and adds `--from-dry-run` flag to apply saved dry-run sessions later.

making use of the assets

running apiservice dry-run for template

What Type of PR Is This?

/kind bug

Related Issue(s)

Fixes 365

Release Notes

Added dry-run mode that saves assets locally without consumer cluster access, and `--from-dry-run` flag to apply saved dry-run sessions.

[mjudeikis]

We need to update all the docs where we use try run.
And if we do bind apiservice --template-name mangodb --dry-run - does it work? if so we need e2e test for it too.

[olamilekan000]

We need to update all the docs where we use try run. And if we do bind apiservice --template-name mangodb --dry-run - does it work? if so we need e2e test for it too.

I haven't tested the dry-run with --template-name, but I can definitely give it a try.
These are the commands I have tried.

    {
      "name": "Launch kubectl-bind dry run",
      "type": "go",
      "request": "launch",
      "mode": "auto",
      "program": "./cli/cmd/kubectl-bind/main.go",
      "args": ["--server", "http://127.0.0.1:8080", "--dry-run", "-o", "yaml"]
    },
    {
      "name": "Apply from dry-run",
      "type": "go",
      "request": "launch",
      "mode": "auto",
      "program": "./cli/cmd/kubectl-bind/main.go",
      "args": ["apiservice", "--from-dry-run", "EW5NQNSL7KPYLHU7DLNXDYTFN6"]
    },

[mjudeikis]

We've been discussing with @xrstf that this might need a bit more thinking.
Maybe we can explisitly force ask the user to provide --dry-output-file to output this, else - instead of this sessionID we give base64 state into screen? Else .kube-bind will get polluted very fast if somebody runs this more often.

Open to other suggestions on how we can change this?

[olamilekan000]

We've been discussing with @xrstf that this might need a bit more thinking. Maybe we can explisitly force ask the user to provide --dry-output-file to output this, else - instead of this sessionID we give base64 state into screen? Else .kube-bind will get polluted very fast if somebody runs this more often.

Open to other suggestions on how we can change this?

What happens when the user wants to apply the binding from the dry-run output? Will the path to the output file be specified using the same --dry-output-file flag, and also, is it the content of the dry-run that'll be base64 encoded?

[mjudeikis]

I think if file provided - dont base64 it, if not - output to the screen in base64? which could be used as input as it.
Would need to model this in cli first maybe before writing more code.

[olamilekan000]

I think if file provided - dont base64 it, if not - output to the screen in base64? which could be used as input as it. Would need to model this in cli first maybe before writing more code.

okay. How soon will you be able to do the modelling?

[mjudeikis]

So been thinking about this. I think simple:

KUBECONFIG=kind-consumer.kubeconfig kubectl bind --dry-run      
DryRun output:
<base64 encoded string>

to use the output:

KUBECONFIG=kind-consumer.kubeconfig kubectl bind --from-dry-run

option 2:

KUBECONFIG=kind-consumer.kubeconfig kubectl bind --dry-run --dry-run-output output.yaml # non base64 at this point
and
KUBECONFIG=kind-consumer.kubeconfig kubectl bind --from-dry-run output.yaml

wdyt? do you see any issues with this way?

[olamilekan000]

So been thinking about this. I think simple:

KUBECONFIG=kind-consumer.kubeconfig kubectl bind --dry-run      
DryRun output:
<base64 encoded string>

to use the output:

KUBECONFIG=kind-consumer.kubeconfig kubectl bind --from-dry-run

option 2:

KUBECONFIG=kind-consumer.kubeconfig kubectl bind --dry-run --dry-run-output output.yaml # non base64 at this point and KUBECONFIG=kind-consumer.kubeconfig kubectl bind --from-dry-run output.yaml

wdyt? do you see any issues with this way?

I think option 2 is better. What happens if no path to the output file is specified? We output to stdout or write to a default file in the current working directory?
In all, for option 2. I see these possibilities:

# Outputs YAML to stdout
KUBECONFIG=kind-consumer.kubeconfig kubectl bind --dry-run

# Writes YAML to a specific file
KUBECONFIG=kind-consumer.kubeconfig kubectl bind --dry-run --dry-run-output=output.yaml

# Dry run with no value for output file. Also just prints yaml to stdout
KUBECONFIG=kind-consumer.kubeconfig kubectl bind --dry-run --dry-run-output

# User can pipe stdout result to a file
KUBECONFIG=kind-consumer.kubeconfig kubectl bind --dry-run --dry-run-output > output.yaml

And can -dry-run-output be shortened to --output and -o as an alias?

[xrstf]

In general I am somewhat unhappy with the current design overall. It seems like the dry run ouput is both

• something on stdout
• a few files
• a few hidden secret files in ~/.kube-bind

And without all of them, re-using the generated state is not possible. Is that correct? If so, I would much prefer if this all yielded instead 1 output thing that is self-contained, like a zip file (as an extreme example) or a big JSON file (potentially encoded to deter people from snooping and fiddling with the state).

[olamilekan000]

In general I am somewhat unhappy with the current design overall. It seems like the dry run ouput is both

• something on stdout
• a few files
• a few hidden secret files in ~/.kube-bind

And without all of them, re-using the generated state is not possible. Is that correct? If so, I would much prefer if this all yielded instead 1 output thing that is self-contained, like a zip file (as an extreme example) or a big JSON file (potentially encoded to deter people from snooping and fiddling with the state).

Yes you're correct but if you look back at the PR thread, you'll see we've already been trying to correct or simplify this.

The initial implementation was built around the requirement in the issue:

This is not a dry run as such, because we basically do more than we say. We should fix this, but for this to work, we will need to store kubeconfig and other related objects locally too, so the user can re-use them in follow-up.

That’s why the dry-run flow ends up producing:

• ouptut to stdout,
• some files stored under the sessionID of the request in the ~/.kube-bind directroy,

In all, I am happy to adjust the PR in any direction that works the best.

[mjudeikis]

-o should be already loaded, so yap, we could reuse it.
I think option 2 as outlined by you make sense. This way we dont touch ~/.kube-bind and all is contained

[mjudeikis]

lets close this for now until we get user signal if we need this or how we do it