Skip to content

Conversation

@iamar7
Copy link
Member

@iamar7 iamar7 commented Jul 30, 2024

Description

#59

Release required?

  • No release
  • Patch release (x.x.X)
  • Minor release (x.X.x)
  • Major release (X.x.x)
Release notes content

Run the pipeline

If the CI pipeline doesn't run when you create the PR, the PR requires a user with GitHub collaborators access to run the pipeline.

Run the CI pipeline when the PR is ready for review and you expect tests to pass. Add a comment to the PR with the following text:

/run pipeline

Checklist for reviewers

  • If relevant, a test for the change is included or updated with this PR.
  • If relevant, documentation for the change is included or updated with this PR.

For mergers

  • Use a conventional commit message to set the release level. Follow the guidelines.
  • Include information that users need to know about the PR in the commit message. The commit message becomes part of the GitHub release notes.
  • Use the Squash and merge option.

@iamar7
Copy link
Member Author

iamar7 commented Jul 30, 2024

/run pipeline

@iamar7
Copy link
Member Author

iamar7 commented Aug 2, 2024

/run pipeline

@iamar7
Copy link
Member Author

iamar7 commented Aug 2, 2024

/run pipeline

@iamar7
Copy link
Member Author

iamar7 commented Aug 4, 2024

/run pipeline

@iamar7
Copy link
Member Author

iamar7 commented Aug 6, 2024

/run pipeline

@iamar7 iamar7 marked this pull request as ready for review August 7, 2024 07:31
@Ak-sky
Copy link
Member

Ak-sky commented Aug 7, 2024

/run pipeline

1 similar comment
@Ak-sky
Copy link
Member

Ak-sky commented Aug 7, 2024

/run pipeline

@iamar7
Copy link
Member Author

iamar7 commented Aug 7, 2024

/run pipeline

@iamar7
Copy link
Member Author

iamar7 commented Aug 7, 2024

/run pipeline

@iamar7
Copy link
Member Author

iamar7 commented Aug 7, 2024

/run pipeline

@iamar7
Copy link
Member Author

iamar7 commented Aug 7, 2024

/run pipeline

@iamar7
Copy link
Member Author

iamar7 commented Aug 9, 2024

/run pipeline

@ocofaigh ocofaigh requested a review from SirSpidey August 15, 2024 10:55
Copy link
Contributor

@SirSpidey SirSpidey left a comment

Choose a reason for hiding this comment

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

some suggestions and comments

ibm_catalog.json Outdated
"key": "cis_id",
"type": "string",
"default_value": "__NULL__",
"description": "Cloud Internet Service ID.",
Copy link
Contributor

Choose a reason for hiding this comment

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

Assuming Conall is correct, include,
...Applies only if secret_manager_public_engine_enabled is true.

Or if always required as he asks, include,
...Required if secret_manager_public_engine_enabled is set to true.

For the rest of the description, is it some ID for the instance or something else. Probably need more details -- perhaps about where to find it if it's not clear to most non-developers.

ibm_catalog.json Outdated
"key": "acme_letsencrypt_private_key",
"type": "string",
"default_value": "__NULL__",
"description": "The private key generated by the ACME account creation tool.",
Copy link
Contributor

Choose a reason for hiding this comment

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

do client in the Cloud catalog know what the ACME tool is? If not, add

For more information, see... and use a markdown link ([]()) to a doc topic about the tool.

@iamar7
Copy link
Member Author

iamar7 commented Aug 21, 2024

/run pipeline

@iamar7
Copy link
Member Author

iamar7 commented Aug 23, 2024

/run pipeline

@iamar7
Copy link
Member Author

iamar7 commented Aug 26, 2024

/run pipeline

SirSpidey
SirSpidey previously approved these changes Aug 26, 2024
Copy link
Contributor

@SirSpidey SirSpidey left a comment

Choose a reason for hiding this comment

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

Looks ok to me. Conall should review for whether it meets his earlier objections.

@SirSpidey
Copy link
Contributor

@iamar7 I approved, but I do have one suggestion.

@iamar7
Copy link
Member Author

iamar7 commented Aug 26, 2024

/run pipeline

@iamar7
Copy link
Member Author

iamar7 commented Sep 3, 2024

/run pipeline

@ocofaigh
Copy link
Contributor

ocofaigh commented Sep 3, 2024

@iamar7 The tests are blocked in this repo due to IBM-Cloud/terraform-provider-ibm#5586
We can add a workaround however we are expecting a fix from the provider soon, so going to wait for that.
This feature is not a high priority so it can wait until after the fix is in.
We already document how consumers can workaround the issue: https://cloud.ibm.com/docs/security-services?topic=security-services-known-issues#ki-scc-attachments

@ocofaigh
Copy link
Contributor

FYI, the pipeline is unblocked here

@iamar7
Copy link
Member Author

iamar7 commented Sep 13, 2024

/run pipeline

@iamar7
Copy link
Member Author

iamar7 commented Sep 13, 2024

/run pipeline

@ocofaigh
Copy link
Contributor

As discussed internally, we are not going to expose all these new variables, but instead document how a user can achieve this by directly update the secrets manager member DA inside the stack.

I think that documentation would live in https://cloud.ibm.com/docs/security-services?topic=security-services-css-relnotes somewhere, so please work with @SirSpidey on that. Since there is no code changes needed in the repo, I'm going to close this PR.

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.

5 participants