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

Add support for AWS STS #978

Merged
merged 10 commits into from
Jul 12, 2024
Merged

Add support for AWS STS #978

merged 10 commits into from
Jul 12, 2024

Conversation

pavolloffay
Copy link
Collaborator

@pavolloffay pavolloffay commented Jul 11, 2024

Resolves #553

https://issues.redhat.com/browse/TRACING-4227

Test instructions:

kubectl create namespace tempo-operator-system
IMG_PREFIX=docker.io/pavolloffay OPERATOR_VERSION=$(date +%s).0.0 BUNDLE_VARIANT=openshift make docker-build docker-push bundle bundle-build bundle-push olm-deploy


kubectl apply -f - <<EOF
apiVersion: v1
kind: Secret
metadata:
  name: aws-sts
stringData:
  bucket: tempoploffay
  region: us-east-2
  role_arn: arn:aws:iam::TBD:role/ploffay-simplest
type: Opaque
EOF


kubectl apply -f - <<EOF
apiVersion: tempo.grafana.com/v1alpha1
kind: TempoStack
metadata:
  name: simplest
spec:
  storage:
    secret:
      name: aws-sts
      type: s3
  storageSize: 1Gi
  resources:
    total:
      limits:
        memory: 2Gi
        cpu: 2000m
  template:
    queryFrontend:
      jaegerQuery:
        enabled: true
        ingress:
          type: route
EOF


kubectl apply -f - <<EOF
apiVersion: tempo.grafana.com/v1alpha1
kind: TempoMonolithic
metadata:
  name: simplest
spec:
  storage:
    traces:
      backend: s3
      s3:
        secret: aws-sts
EOF

Signed-off-by: Pavol Loffay <p.loffay@gmail.com>
Signed-off-by: Pavol Loffay <p.loffay@gmail.com>
@codecov-commenter
Copy link

codecov-commenter commented Jul 11, 2024

Codecov Report

Attention: Patch coverage is 73.39450% with 29 lines in your changes missing coverage. Please review.

Project coverage is 73.23%. Comparing base (75de22c) to head (3ac2873).
Report is 1 commits behind head on main.

Files Patch % Lines
internal/manifests/monolithic/configmap.go 38.88% 7 Missing and 4 partials ⚠️
internal/handlers/storage/secret.go 84.61% 5 Missing and 1 partial ⚠️
cmd/generate/main.go 0.00% 4 Missing ⚠️
internal/manifests/manifestutils/annotations.go 0.00% 4 Missing ⚠️
internal/handlers/storage/storage.go 0.00% 2 Missing ⚠️
internal/manifests/config/build.go 0.00% 1 Missing ⚠️
internal/manifests/manifests.go 0.00% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main     #978   +/-   ##
=======================================
  Coverage   73.22%   73.23%           
=======================================
  Files         105      105           
  Lines        6503     6568   +65     
=======================================
+ Hits         4762     4810   +48     
- Misses       1450     1465   +15     
- Partials      291      293    +2     
Flag Coverage Δ
unittests 73.23% <73.39%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Signed-off-by: Pavol Loffay <p.loffay@gmail.com>
Signed-off-by: Pavol Loffay <p.loffay@gmail.com>
Signed-off-by: Pavol Loffay <p.loffay@gmail.com>
Signed-off-by: Pavol Loffay <p.loffay@gmail.com>
@IshwarKanse
Copy link
Contributor

@pavolloffay With the route enabled in TempoStack query frontend, we create a SA for the query frontend along with the default SA for the stack. The query frontend SA doesn't have the required annotations for STS which causes the frontend pod to fail.

apiVersion: tempo.grafana.com/v1alpha1
kind: TempoStack
metadata:
  name: tmstack
spec:
  storage:
    secret:
      name: aws-sts
      type: s3
  storageSize: 20Gi
  resources:
    total:
      limits:
        memory: 4Gi
        cpu: 2000m
  template:
    queryFrontend:
      jaegerQuery:
        enabled: true
        ingress:
          type: route

oc get sa
NAME                           SECRETS   AGE
builder                        1         38m
default                        1         38m
deployer                       1         38m
tempo-tmstack                  1         29m
tempo-tmstack-query-frontend   1         29m

% oc get sa tempo-tmstack-query-frontend -o yaml
apiVersion: v1
imagePullSecrets:
- name: tempo-tmstack-query-frontend-dockercfg-7j4cm
kind: ServiceAccount
metadata:
  annotations:
    serviceaccounts.openshift.io/oauth-redirectreference.primary: '{"kind":"OAuthRedirectReference","apiVersion":"v1","reference":{"kind":"Route","name":"tempo-tmstack-query-frontend"}}'
  creationTimestamp: "2024-07-12T04:39:44Z"
  labels:
    app.kubernetes.io/component: query-frontend
    app.kubernetes.io/instance: tmstack
    app.kubernetes.io/managed-by: tempo-operator
    app.kubernetes.io/name: tempo
  name: tempo-tmstack-query-frontend
  namespace: test-tempostack
  ownerReferences:
  - apiVersion: tempo.grafana.com/v1alpha1
    blockOwnerDeletion: true
    controller: true
    kind: TempoStack
    name: tmstack
    uid: e98d32a4-b942-4e66-b624-db57574bc78f
  resourceVersion: "128365"
  uid: 95134fd8-0f67-41ac-baa0-88475240973c
secrets:
- name: tempo-tmstack-query-frontend-dockercfg-7j4cm

 % oc get pods
NAME                                           READY   STATUS             RESTARTS       AGE
tempo-tmstack-compactor-67776478c9-h7rs5       1/1     Running            0              32m
tempo-tmstack-distributor-5f8d5f9655-x8qph     1/1     Running            0              32m
tempo-tmstack-ingester-0                       1/1     Running            0              32m
tempo-tmstack-querier-859d965f78-d2p44         1/1     Running            0              32m
tempo-tmstack-query-frontend-599769fdd-xlpwz   2/3     CrashLoopBackOff   11 (69s ago)   32m

Signed-off-by: Pavol Loffay <p.loffay@gmail.com>
Signed-off-by: Pavol Loffay <p.loffay@gmail.com>
Signed-off-by: Pavol Loffay <p.loffay@gmail.com>
Signed-off-by: Pavol Loffay <p.loffay@gmail.com>
@pavolloffay
Copy link
Collaborator Author

The issue is that the query-frontend uses a different service account when the route for the Jaeger UI is enabled.

tempo-tmstack                  1         6m25s
tempo-tmstack-query-frontend   1         6m24s

@IshwarKanse could we require associating role with two SAs? Lokistack does it https://loki-operator.dev/docs/short_lived_tokens_authentication.md/#aws-secure-token-service

# Create a trust relationship file
cat > "$trust_rel_file" <<EOF
{
  "Version": "2012-10-17",
  "Statement": [
    {
      "Effect": "Allow",
      "Principal": {
        "Federated": "arn:aws:iam::${aws_account_id}:oidc-provider/${oidc_provider}"
      },
      "Action": "sts:AssumeRoleWithWebIdentity",
      "Condition": {
        "StringEquals": {
          "${oidc_provider}:sub": [
            "system:serviceaccount:${tempostack_ns}:tempo-${tempostack_name}",
           "system:serviceaccount:${tempostack_ns}:tempo-${tempostack_name}-query-frontend"
         ]
       }
     }
   }
 ]
}
EOF

@IshwarKanse
Copy link
Contributor

@pavolloffay Tested with the fix commit. The stack is running now. I have also added the AWS IAM policy script to our git repo. https://github.com/openshift/distributed-tracing-qe/blob/main/scripts/aws-sts-s3-access.sh

@pavolloffay pavolloffay merged commit 5d1772f into grafana:main Jul 12, 2024
11 checks 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.

Setup: tempo-operator with AWS IRSA / IAM
5 participants