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

S163 docs review #592

Draft
wants to merge 4 commits into
base: rc-v0.5.0
Choose a base branch
from
Draft

S163 docs review #592

wants to merge 4 commits into from

Conversation

davidfq
Copy link
Contributor

@davidfq davidfq commented Dec 4, 2023

Fixes

  • Minor GitHub TODO issues
  • Hardcoded secret key prefixes in TODO files that don't match actual keys; only for GitHub, if change is OK I'll fix the rest of the specs.

Features

.

Change implications

  • dependencies added/changed? no
  • something to note in CHANGELOG.md? no
    • anything that will show up in terraform plan/apply that isn't obviously a no-op? no
    • breaking changes? if in module/example that is NOT marked alpha, requires major version change no

@davidfq
Copy link
Contributor Author

davidfq commented Dec 4, 2023

@aperez-worklytics AWS stuff complaining... I made a couple tries, but not able to fix missing config_parameter_prefix

@davidfq davidfq marked this pull request as draft December 4, 2023 16:27
Base automatically changed from rc-v0.4.42 to main December 6, 2023 21:21
@eschultink eschultink changed the base branch from main to rc-v0.4.43 December 6, 2023 22:03
- `PSOXY_GITHUB_CLIENT_ID` with `App ID` value. **NOTE**: It should be `App Id` value as we are going to use authentication through the App and **not** *client_id*.
- `PSOXY_GITHUB_PRIVATE_KEY` with content of the `gh_pk_pkcs8.pem` from previous step. You could open the certificate with VS Code or any other editor and copy all the content *as-is* into this variable.
- `${var.config_parameter_prefix}GITHUB_CLIENT_ID` with `App ID` value. **NOTE**: It should be `App Id` value as we are going to use authentication through the App and **not** *client_id*.
- `${var.config_parameter_prefix}GITHUB_PRIVATE_KEY` with content of the `gh_pk_pkcs8.pem` from previous step. You could open the certificate with VS Code or any other editor and copy all the content *as-is* into this variable.
Copy link
Member

Choose a reason for hiding this comment

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

Yes, this is something we need to solve but it's complicated. The value you want here is specific to each proxy deployment.

By default, it's PSOXY_GITHUB_ for the whole thing; but this is just what's for the default case.

Both the PSOXY_ prefix can differ and the GITHUB_ prefix for the lambda (cloud function).

  • The former is to support customers putting the SSM parameters in some hierachy, esp when deploying to a shared AWS account; they might use people-analytics/worklytics/ for example in AWS case (which conventionally uses /-style hierarchies).
  • The latter may vary if multiple instances of GitHub connectors for example, which we want/need to support. in which case they may have several distinct proxy deployments of the same source kind. (eg, we have people who have multiple GitHub organizations, such as one for their open source stuff, and another one for in-house)

I've tried to solve, but the logic that we use to generate the prefix value is in the aws-psoxy-rest/gcp-psoxy-rest modules atm, and as I recall I get cyclic dependency if I try to output from there to fill as a variable to worklytics-connectors- modules. Avoiding that may be a bigger refactor - splitting out the logic that defines the identifiers for each proxy instance independently at the top level of our examples, at the cost of adding an additional clone of the repository to the dependency footprint.

Other approach is to leave this parameterized, and the value of external_token_todo is a template to be evaluated later, once the

Base automatically changed from rc-v0.4.43 to main December 20, 2023 18:49
@eschultink eschultink changed the base branch from main to rc-v0.4.44 December 21, 2023 03:17
Base automatically changed from rc-v0.4.44 to main January 2, 2024 23:16
@eschultink eschultink changed the base branch from main to rc-v0.4.45 January 9, 2024 21:36
Base automatically changed from rc-v0.4.45 to main January 16, 2024 15:57
@eschultink eschultink changed the base branch from main to rc-v0.4.46 January 16, 2024 16:51
Base automatically changed from rc-v0.4.46 to main February 1, 2024 18:33
@eschultink eschultink changed the base branch from main to rc-v0.4.47 February 1, 2024 19:13
Base automatically changed from rc-v0.4.47 to main February 10, 2024 00:24
@eschultink eschultink changed the base branch from main to rc-v0.4.48 February 12, 2024 17:44
Base automatically changed from rc-v0.4.48 to main February 16, 2024 22:19
@eschultink eschultink changed the base branch from main to rc-v0.4.50 March 5, 2024 18:10
Base automatically changed from rc-v0.4.50 to main March 6, 2024 23:09
@eschultink eschultink changed the base branch from main to rc-v0.4.51 March 7, 2024 03:48
Base automatically changed from rc-v0.4.51 to main March 23, 2024 16:48
@eschultink eschultink changed the base branch from main to rc-v0.4.52 March 25, 2024 16:32
Base automatically changed from rc-v0.4.52 to main April 16, 2024 15:26
@eschultink eschultink changed the base branch from main to rc-v0.4.53 April 24, 2024 02:34
Base automatically changed from rc-v0.4.53 to main May 22, 2024 17:08
@eschultink eschultink changed the base branch from main to rc-v0.4.54 May 22, 2024 17:17
Base automatically changed from rc-v0.4.54 to main June 3, 2024 21:39
@eschultink eschultink changed the base branch from main to rc-v0.4.55 June 3, 2024 22:35
Base automatically changed from rc-v0.4.55 to main July 1, 2024 23:22
@eschultink eschultink changed the base branch from main to rc-v0.4.56 July 2, 2024 13:23
Base automatically changed from rc-v0.4.56 to main July 4, 2024 10:44
@eschultink eschultink changed the base branch from main to rc-0.4.57 July 5, 2024 17:51
Base automatically changed from rc-0.4.57 to main July 11, 2024 21:27
@eschultink eschultink changed the base branch from main to rc-v0.4.58 July 11, 2024 21:55
Base automatically changed from rc-v0.4.58 to main August 19, 2024 15:36
@eschultink eschultink changed the base branch from main to rc-v0.4.61 September 19, 2024 18:11
Base automatically changed from rc-v0.4.61 to main October 3, 2024 20:41
@eschultink eschultink changed the base branch from main to rc-v0.5.0 October 3, 2024 21:24
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.

2 participants