Skip to content
This repository has been archived by the owner on Jul 10, 2024. It is now read-only.

Add command to generate Kubernetes Secrets #202

Merged
merged 25 commits into from
Mar 21, 2023

Conversation

versilis
Copy link
Contributor

@versilis versilis commented Mar 15, 2023

This adds a new command akita kube secret which generates a Kubernetes secret configuration file that stores a user's base-64 encoded Akita API credentials.

To simplify file generation, I've used go's built-in templating utilities; akita-secret.tmpl is used as the template for creating the secret.

Example usage:

% ./bin/akita kube secret -n test -o ./tmp/deployments/configs/akita-secrets.yml
[INFO] Akita Agent 0.0.0
[INFO] Generated Kubernetes secret config to ./tmp/deployments/configs/akita-secrets.yml                                                                                                               

% less ./tmp/deployments/configs/akita-secrets.yml 
apiVersion: v1
kind: Secret
metadata:
  name: akita-secrets
  namespace: test
type: Opaque
data:
  akita-api-key: YXBrXzN1Y2h6WDyiOdyMzg2NldiM3Y0Q2Y=
  akita-api-secret: YmUzMzY5OTllNzNjc2DY3MmI5OWQzMTVmAnGaKmYzN2NlNjc2NWRiZDY4MzNjMWRkMzA4YjFjZDFlNWZkZg==
./tmp/deployments/configs/akita-secrets.yml lines 1-9/9 (END)

@versilis versilis force-pushed the versilis/kube-secret branch from b7f4f96 to 3226cf1 Compare March 15, 2023 20:00
@versilis versilis force-pushed the versilis/kube-secret branch from 7202b3f to 10ad02d Compare March 15, 2023 22:23
@versilis versilis self-assigned this Mar 15, 2023
@versilis versilis changed the base branch from main to versilis/kube March 15, 2023 22:34
@versilis versilis changed the title Add Kube Secret Command Add command to generate Kubernetes Secrets Mar 16, 2023
@versilis versilis requested a review from mgritter March 16, 2023 02:37
@versilis versilis marked this pull request as ready for review March 16, 2023 02:37
@versilis versilis added the 3 – Normal Priority Non-blocking review—please turn around quickly label Mar 16, 2023
Copy link
Contributor

@mgritter mgritter left a comment

Choose a reason for hiding this comment

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

Looks pretty good.

The must-fix items: let's add the ability to write to stdout, and use the default namespace. (And the redundant code in addAgentToECS.)

I would like to see us tackle YAML output with a library here on an easy case before we have to tackle parsing and rewriting a Deployment resource. Unless you were planning on using just string manipulation? But I don't insist on it.

cmd/internal/ecs/ecs.go Outdated Show resolved Hide resolved
cmd/internal/kube/kube.go Outdated Show resolved Hide resolved
cmd/internal/kube/secret.go Outdated Show resolved Hide resolved
cmd/internal/kube/secret.go Outdated Show resolved Hide resolved
cmd/internal/kube/template/akita-secret.tmpl Show resolved Hide resolved
cmd/internal/kube/secret.go Outdated Show resolved Hide resolved
)
_ = secretCmd.MarkFlagRequired("namespace")

secretCmd.Flags().StringVarP(&output, "output", "o", "akita-secret.yml", "File to output the generated secret.")
Copy link
Contributor

Choose a reason for hiding this comment

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

This should have come up in the design review -- sorry. I think the correct default, which is most idiomatic to Kubernetes tools, is to print to standard output.

The idiomatic usage we are aiming for is something like "akita kube secret | kubectl apply -f -"

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've implemented printing in 5acfe06.

To avoid printing pre-run info logs, I've overriden the root command's PersistentPreRun handler. I've also had to make some updates to how we initialize telemetry which I've added as part of this commit: df81990

func init() {
var err error

secretTemplate, err = template.ParseFS(templateFS, "template/akita-secret.tmpl")
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm mostly OK with using a template here. But, I think this will be much harder to pull off for the next command, and I would like the two implementations to be consistent. If we have to have YAML parsing and output, let's start here with the easy case. :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Currently, there's a discrepancy between the Kubernetes openapi spec and the Go packages API model representation. Here's a link to the issue that covers it: kubernetes/kubernetes#109427

I've opened a separate PR to address using the Kubernetes API for generating secrets: #204

cmd/internal/kube/secret.go Outdated Show resolved Hide resolved
cmd/internal/kube/kube.go Show resolved Hide resolved
Comment on lines 39 to 40
// Output the generated secret to the console
printer.RawOutput(output)
Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, I think I was unclear. The two uses cases are:

  1. Apply directly
    akita kube secret | kubectl apply -f -

  2. Apply via a file (convenience, they could always pipe to a file.)
    akita kube secret -f mysecret.yaml
    kubectl apply -f mysecret.yaml

In case #2 we should not print to standard output as well, it should be one or the other. We can support this in a few different ways, I don't much care whether (a) standard out is the default if -f not specified, or (b) -f - writes to standard output..

I kind of think like in case #1 we should not write the file that was not asked for as well.

Come talk to me if what should happen in these two cases are still unclear.

}

// Creates a file at the give path to be used for storing of the generated Secret config
// If any child dicrectories do not exist, it will be created.
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this comment correct? I think you mean "it will not be created".

@@ -127,6 +128,7 @@ func runTCPFlowTestCase(c tcpFlowTestCase) error {
}

func TestTCPFlow(t *testing.T) {
telemetry.Init(false)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think I understand the need here and in pase_http_test since the error functions in parsing send telemetry.

Is there a fix to telemetry that checks whether uninitialized instead? If it's not easy to do, we can keep this, it just seems a bit odd to be initializing telemetry in a situation where we don't really want it.

Copy link
Contributor

@mgritter mgritter left a comment

Choose a reason for hiding this comment

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

Looks good!

@versilis versilis merged commit a87238f into versilis/kube Mar 21, 2023
@versilis versilis deleted the versilis/kube-secret branch March 21, 2023 23:52
This was referenced Mar 24, 2023
versilis added a commit that referenced this pull request Mar 27, 2023
This adds two new commands, `akita kube inject` and `akita kube secret`,
for simplifying the process of installing Akita as a sidecar in
Kubernetes Deployments.

Changes include:
- #202
- #207
- #206
---------

Signed-off-by: versilis <[email protected]>
Co-authored-by: Mark Gritter <[email protected]>
Co-authored-by: Jed Liu <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
3 – Normal Priority Non-blocking review—please turn around quickly
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants