k8sgpt icon indicating copy to clipboard operation
k8sgpt copied to clipboard

feature: Allow openai token to be specified via command/env variable

Open patrickpichler opened this issue 1 year ago • 3 comments

Checklist:

  • [x] I've searched for similar issues and couldn't find anything matching
  • [x] I've discussed this feature in the #k8sgpt slack channel

Is this feature request related to a problem?

  • [ ] Yes
  • [x] No

Describe the solution you'd like

Having password in config files is not always what a certain user of the application wants. I propose a way of specifying any command instead of the plain string, which output will then be treated as the password.

For this the current config file structure needs to change a bit. Here an example:

ai:
    providers:
        - name: openai1
          password: my-secure-password
        - name: openai2
          model: gpt-3.5-turbo
          password:
            type: value
            value: my-secure-password
        - name: openai3
          model: gpt-3.5-turbo
          password:
            type: command
            command:
            - echo
            - my-secure-password       
          - name: openai4
          model: gpt-3.5-turbo
          password:
            type: environment
            name: OPENAI_TOKEN

Here we can see how this could look like.

The password field should be transformed from a simple string, to an object. The type parameter specifies how the rest of the yaml map shall be interpreted. I would have 3 types in mind:

  • value
  • environment
  • command

For type value, password value would be specified by a field value. There should also be a short form, which is depicted in openai1 in the example above. This is basically the current way of defining password and would ensure no breaking change in the config.

Type environment would require a name field, which specifies the name of the environment variable holding the token.

Type command requires a command field, which is a list of string, defining a command + arguments to be called in order to retrieve the password.

Benefits for the project and its users

This would improve the security for end users, since they are no longer required to store their api tokens in the configuration file.

The design is also extensible, should the need arise to add new password types.

Potential drawbacks

The only downside I can think of is code execution when running untrusted configs. I am not 100% convinced that this is a real issue though.

Additional context

patrickpichler avatar Apr 10 '23 19:04 patrickpichler

@thschue @AlexsJones what are your opinions on this?

patrickpichler avatar Apr 10 '23 19:04 patrickpichler

I like the idea, we just need to ensure that it’s transparent for the user, e.g. the auth command behaves as now and additional things can be used via params in the cli. There’s also an issue for encryption in Progress (#224), which should be kept in mind … From my side, there are no objections, would love to see this 👍

thschue avatar Apr 10 '23 20:04 thschue

Can you illustrate how this would look in the CLI?

AlexsJones avatar Apr 11 '23 09:04 AlexsJones

Just to close this thought out, it's possible with Viper to env reading, which we actually already do.


	viper.SetEnvPrefix("K8SGPT")
	viper.AutomaticEnv() // read in environment variables that match

This means you'd be able to use the token such as

	AnalyzeCmd.Flags().BoolVarP(&anonymize, "anonymize", "a", false, "Anonymize data before sending it to the AI backend. This flag masks sensitive data, such as Kubernetes object names and labels, by replacing it with a key. However, please note that this flag does not currently apply to events.")
	// array of strings flag
	AnalyzeCmd.Flags().StringSliceVarP(&filters, "filter", "f", []string{}, "Filter for these analyzers (e.g. Pod, PersistentVolumeClaim, Service, ReplicaSet)")
	// explain flag
	AnalyzeCmd.Flags().BoolVarP(&explain, "explain", "e", false, "Explain the problem to me")
	

Would become K8SGPT_EXPLAIN etc..

The issue is that we don't auth and analyze in one command.

So I think the real work is modifying the analyze command to detect if those params are set and do a "dynamic" auth

AlexsJones avatar Apr 28 '23 06:04 AlexsJones