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

Update: service account for capa and aws provider on irsa #230

Merged
merged 9 commits into from
Jan 13, 2023

Conversation

zewolfe
Copy link
Contributor

@zewolfe zewolfe commented Jan 10, 2023

This PR:
Updates the service account annotation for irsa on aws and capa

Towards - giantswarm/roadmap#1640


Checklist

  • Added a CHANGELOG entry

Testing

The instance of external-dns installed as part of Giant Swarm platform releases watches services in the kube-system namespace with annotations giantswarm.io/external-dns=managed and external-dns.alpha.kubernetes.io/hostname matching the clusters base domain. (You can find this in the deployments args --domain-filter value)

You can take this example Service, apply it to your cluster. Change the external-dns.alpha.kubernetes.io/hostname annotation to match your clusters base domain.

then:

  • Check external-dns logs for lines like Desired change: CREATE test.your.configured.domain.gigantic.io CNAME
  • Try to resolve the domain (https://www.dnstester.net/)
apiVersion: v1
kind: Service
metadata:
  annotations:
    external-dns.alpha.kubernetes.io/hostname: test.your.configured.domain.gigantic.io
    external-dns.alpha.kubernetes.io/ttl: "60"
    giantswarm.io/external-dns: managed
  name: test-external-dns
  namespace: kube-system
spec:
  type: ExternalName
  externalName: www.giantswarm.io

For testing upgrades:

  • Create the service and check for creation
  • Upgrade
  • Delete the service and check for deletion

Default app on AWS releases

  • Fresh install works
  • Upgrade works

Default app on Azure releases

  • Fresh install works
  • Upgrade works

Optional app (KVM)

  • Fresh install works
  • Upgrade works

@zewolfe zewolfe marked this pull request as ready for review January 10, 2023 16:57
@zewolfe zewolfe requested a review from a team as a code owner January 10, 2023 16:57
# aws.accountID
# AWS account ID is used to assume role via IRSA (IAM roles for service accounts).
# It is dynamically set and will be overridden.
accountID: ""
Copy link
Member

Choose a reason for hiding this comment

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

we still need this for non vintage clusters

{{- $_ := set .Values.serviceAccount.annotations "eks.amazonaws.com/role-arn" (tpl "arn:aws:iam::{{ .Values.aws.accountID }}:role/{{ template \"aws.iam.role\" . }}" .) }}
{{- end}}
{{- if and (or (eq .Values.provider "capa")) }}
Copy link
Member

Choose a reason for hiding this comment

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

We still need to verify aws access internal. The reason is customer can also use "external" which uses ENV to set AWS credentials

Suggested change
{{- if and (or (eq .Values.provider "capa")) }}
{{- else if and (eq .Values.provider "capa") (eq .Values.aws.access "internal") }}

@@ -11,9 +11,6 @@
"access": {
"type": "string"
},
"accountID": {
Copy link
Member

Choose a reason for hiding this comment

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

Can you please add that back? Thank you!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch. Thanks!

Copy link
Member

@njuettner njuettner left a comment

Choose a reason for hiding this comment

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

lgtm

Copy link
Contributor

@mcharriere mcharriere 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!

@zewolfe zewolfe merged commit e66d78e into master Jan 13, 2023
@zewolfe zewolfe deleted the update-sa-for-irsa branch January 13, 2023 09:06
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.

None yet

4 participants