Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: improve default_prompt #406

Merged
merged 5 commits into from
May 16, 2023

Conversation

camigira
Copy link
Contributor

@camigira camigira commented May 14, 2023

📑 Description

(closes: #417)

Attempt to improve the default_prompt aiming to:

  • Prevent Prompt Injection by specifying variable text win triple dashes -.
  • Keep output concise but insightful.
  • Clearer output Error vs Solution.

Original Prompt
image

New Prompt
image

Note: After a few tests, the text Provide the most possible solution seemed to be better than Provide the most likely solution as it explored multiple options as opposed to focusing the answer on a single option.

✅ Checks

  • My pull request adheres to the code style of this project
  • My code requires changes to the documentation
  • I have updated the documentation as required
  • All the tests have passed

ℹ Additional Information

@camigira camigira requested review from a team as code owners May 14, 2023 06:30
@camigira camigira force-pushed the fix/prevent-prompt-injection branch from f5bb9a8 to 5cae455 Compare May 14, 2023 06:46
@camigira camigira changed the title Improve default prompt fix: Improve default prompt May 14, 2023
@camigira camigira changed the title fix: Improve default prompt fix: improve default prompt May 14, 2023
@camigira camigira changed the title fix: improve default prompt fix: Improve default_prompt May 14, 2023
Signed-off-by: Camilo Giraldo <camigira@gmail.com>
@camigira camigira force-pushed the fix/prevent-prompt-injection branch from 5cae455 to 9e74b6a Compare May 14, 2023 06:52
@camigira camigira changed the title fix: Improve default_prompt fix: improve default_prompt May 14, 2023
@matthisholleville
Copy link
Contributor

I wonder how the prompt behaves in case there are several errors on an object? Could you please provide an example with this case?

Signed-off-by: Camilo Giraldo <camigira@gmail.com>
@camigira
Copy link
Contributor Author

Example for a broken pod and deployment with multiple issues (hope this is what you had in mind with your question)

image

broken-pod.yml

apiVersion: v1
kind: Pod
metadata:
  name: broken-pod
spec:
  containers:
    - name: broken-pod
      image: nginx:1.a.b.c
      livenessProbe:
        httpGet:
          path: /
          port: 90
        initialDelaySeconds: 3
        periodSeconds: 3

broken-deployment.yml

apiVersion: apps/v1
kind: Deployment
metadata:
  name: broken-deployment
spec:
  replicas: 3
  selector:
    matchLabels:
      app: myapp
  template:
    metadata:
      labels:
        app: myapp
    spec:
      containers:
        - name: nginx-container
          image: nginx:latest
          ports:
            - containerPort: 80
          livenessProbe:
            httpGet:
              path: /healthz #doesn't exist in nginx
              port: 80
            initialDelaySeconds: 30
            periodSeconds: 10
          readinessProbe:
            httpGet:
              path: /readiness #doesn't exist in nginx
              port: 80
            initialDelaySeconds: 20
            periodSeconds: 5

@matthisholleville
Copy link
Contributor

matthisholleville commented May 14, 2023

You still have only one error per object.

Please try with this object:

apiVersion: networking.k8s.io/v1
kind: Ingress
metadata:
  name: fake-ingress
spec:
  rules:
  - host: fake-ingress.example.com
    http:
      paths:
      - backend:
          service:
            name: fake-service
            port:
              number: 80
        path: /
        pathType: Prefix
  tls:
  - hosts:
    - fake-ingress.example.com
    secretName: fake-secret

You should have this result :

0 k8sgpt/fake-ingress(fake-ingress)
- Error: Ingress k8sgpt/fake-ingress does not specify an Ingress class.
- Error: Ingress uses the service k8sgpt/fake-service which does not exist.
- Error: Ingress uses the secret k8sgpt/fake-secret as a TLS certificate which does not exist.

@camigira
Copy link
Contributor Author

Thanks @matthisholleville for providing the example, here the results:

Current Default Prompt

0 default/fake-ingress(fake-ingress)
- Error: Ingress default/fake-ingress does not specify an Ingress class.
- Error: Ingress uses the service default/fake-service which does not exist.
- Error: Ingress uses the secret default/fake-secret as a TLS certificate which does not exist.
The Ingress resource is trying to use a service and a TLS certificate that do not exist, and it also does not specify which Ingress class to use.

To fix this error, create the necessary service and secret resources, and specify the Ingress class to use in the Ingress resource.

Proposed Default Prompt

0 default/fake-ingress(fake-ingress)
- Error: Ingress default/fake-ingress does not specify an Ingress class.
- Error: Ingress uses the service default/fake-service which does not exist.
- Error: Ingress uses the secret default/fake-secret as a TLS certificate which does not exist.
Error: Ingress default/fake-ingress needs an Ingress class, service default/fake-service doesn't exist, and secret default/fake-secret is a non-existent TLS cert.

Solution: 
1. Create a service named default/fake-service.
2. Create a secret named default/fake-secret with your TLS cert.
3. Add an ingress class to your Ingress resource, like nginx or traefik. 
4. Update your Ingress resource to use the service and secret created in step 1 and 2.

@matthisholleville
Copy link
Contributor

I really like the solution part @camigira but the error part in the prompt seems redondant.

What do you think @AlexsJones ?

@AlexsJones
Copy link
Member

I really like the solution part @camigira but the error part in the prompt seems redondant.

What do you think @AlexsJones ?

I think as long as it yields the outputs that are clear and understandable to the user its good.
I don't know whether the error being specified in the block improves the prompt at all - we need to try with additional backends to be sure.

@camigira
Copy link
Contributor Author

👋 @AlexsJones @matthisholleville

I saw this PR was approved but there are still unresolved conversations. Should I mark as completed and merge?

Thanks!

@AlexsJones AlexsJones enabled auto-merge (squash) May 16, 2023 06:29
@AlexsJones AlexsJones merged commit 06542b4 into k8sgpt-ai:main May 16, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

[Feature]: Improve Default Prompt
3 participants