Skip to content

Conversation

@bankhubjar
Copy link

No description provided.

@gemini-code-assist
Copy link

Summary of Changes

Hello @bankhubjar, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request introduces a standardized and reusable mechanism for defining Kubernetes ServiceAccounts across Helm charts. By creating a common ServiceAccount template and integrating it into the onechart chart, it streamlines the process of configuring service identities for applications, promoting consistency and reducing duplication.

Highlights

  • New Common ServiceAccount Template: A new Helm template, _serviceaccount.yaml, has been added to the charts/common directory. This template defines a Kubernetes ServiceAccount resource, allowing for its conditional creation based on chart values.
  • Conditional ServiceAccount Creation: The newly introduced common service account template enables the creation of a ServiceAccount only if .Values.serviceaccount is explicitly set, providing flexibility in deployment configurations.
  • Integration with Onechart: The onechart Helm chart now includes the common service account template via charts/onechart/templates/serviceaccount.yaml. This ensures that onechart can leverage the standardized ServiceAccount definition from the common chart.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request adds a ServiceAccount resource to the onechart Helm chart by creating a reusable template in the common library chart. The implementation has a critical bug due to a case-sensitivity issue that will prevent it from working correctly. I've provided a suggestion to fix this bug and refactor the implementation to follow common Helm chart best practices, making it more flexible and robust by using a structured configuration value. This change also includes adding standard labels and using a generated name by default, which are recommended practices.

Comment on lines +2 to +9
{{- if .Values.serviceaccount }}
---
apiVersion: v1
kind: ServiceAccount
metadata:
name: {{ .Values.serviceAccount }}
namespace: {{ .Release.Namespace }}
{{- end }}

Choose a reason for hiding this comment

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

high

This implementation has a critical bug and could be made more flexible and robust.

  1. Bug: There is a case mismatch between .Values.serviceaccount in the if condition and .Values.serviceAccount used for the name. This will prevent the template from working as intended.
  2. Inflexible Design: Using a single value to both enable creation and provide the name is not ideal. For example, if a user sets serviceAccount: true, the resource name would be invalid. A structured object is the standard practice in Helm charts.

I recommend refactoring this to use a structured value. This also makes it easy to add labels and use a default name for the service account, improving consistency with other resources. This would require adding the following to onechart/values.yaml:

serviceAccount:
  create: false
  # name: my-service-account

Note: The suggestion below uses onechart.fullname and onechart.labels. Since this template is in a library chart, hardcoding the parent chart name (onechart) is not ideal for reusability. A more advanced approach would be needed for a truly generic template, but this is a good improvement for the current use case.

{{- if .Values.serviceAccount.create -}}
---
apiVersion: v1
kind: ServiceAccount
metadata:
  name: {{ .Values.serviceAccount.name | default (include "onechart.fullname" .) }}
  namespace: {{ .Release.Namespace }}
  labels:
    {{- include "onechart.labels" . | nindent 4 }}
{{- end }}

@bankhubjar bankhubjar merged commit 08b9abe into opsta:master Oct 10, 2025
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

None yet

Development

Successfully merging this pull request may close these issues.

1 participant