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.

Sign up for GitHub

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

Jump to bottom

Attestation service deploy updates + support attestation signers #1797

Merged
merged 6 commits into from
Nov 21, 2019
Merged

Attestation service deploy updates + support attestation signers #1797

merged 6 commits into from
Nov 21, 2019

Conversation

nambrot
Copy link
Contributor

@nambrot nambrot commented Nov 20, 2019 •
edited
Loading

Description

This PR makes some minor updates to the deploy of our attestation service and corresponding fixes:

  • It tries to fetch the account address at startup to highlight if it wasn't configured
  • It by default makes CLabs validators register a separate attestation key
  • It fixes a contractkit bug where we would assume that the account address is the same as the address signing the attestation
  • It updates the helm chart to our latest configuration

Tested

  • on namoffchainreveal

Related issues

@nambrot nambrot self-assigned this Nov 20, 2019
@timmoreton timmoreton added the Priority: P0 Blocker label Nov 20, 2019
@nambrot nambrot added betanet-blocker and removed Priority: P0 Blocker labels Nov 20, 2019
@codecov
Copy link

codecov bot commented Nov 20, 2019 •
edited
Loading

Codecov Report

Merging #1797 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff           @@
##           master   #1797   +/-   ##
======================================
  Coverage    74.2%   74.2%           
======================================
  Files         278     278           
  Lines        7653    7653           
  Branches      672     672           
======================================
  Hits         5679    5679           
  Misses       1857    1857           
  Partials      117     117
Flag Coverage Δ
#mobile 74.2% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9f5e421...b919494. Read the comment docs.

@nambrot nambrot changed the title Nambrot/update attestation service deploy Attestation service deploy updates + support attestation signers Nov 20, 2019
@nambrot nambrot assigned timmoreton and asaj and unassigned nambrot Nov 20, 2019
timmoreton
timmoreton approved these changes Nov 21, 2019
View reviewed changes
packages/helm-charts/attestation-service/templates/attestation.statefulset.yaml
@@ -78,7 +80,11 @@ spec:
- name: CELO_PROVIDER
value: https://{{ .Release.Namespace }}-forno.{{ .Values.domain.name }}.org
- name: APP_SIGNATURE
value: {{ .Values.attestation_service.sms_retriever_hash_code }}}
value: {{ .Values.attestation_service.sms_retriever_hash_code }}
- name: SMS_PROVIDERS
Copy link
Contributor

Choose a reason for hiding this comment

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

presuming this arrived in another PR

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup

@nambrot nambrot added the automerge Have PR merge automatically when checks pass label Nov 21, 2019
@asaj asaj assigned nambrot and unassigned timmoreton and asaj Nov 21, 2019
@celo-ci-bot-user celo-ci-bot-user merged commit 6dea5b9 into master Nov 21, 2019
@celo-ci-bot-user celo-ci-bot-user deleted the nambrot/update-attestation-service-deploy branch November 21, 2019 02:31
@leojoy95 leojoy95 mentioned this pull request Oct 24, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@timmoreton timmoreton timmoreton approved these changes

@asaj asaj Awaiting requested review from asaj

@m-chrzan m-chrzan Awaiting requested review from m-chrzan

Assignees

@nambrot nambrot

Labels
automerge Have PR merge automatically when checks pass
Projects
None yet
Milestone
No milestone
4 participants
@nambrot @timmoreton @asaj @celo-ci-bot-user