Skip to content
This repository has been archived by the owner on Jan 9, 2021. It is now read-only.

Updates instrux in README, CONTRIBUTING and app notes #44

Merged
merged 1 commit into from
Apr 28, 2020

Conversation

diverdane
Copy link
Contributor

@diverdane diverdane commented Apr 13, 2020

Changes included:

  • A VERSION text file is added to set the current published version
    for the Marketplace app.
  • The publishedVersion field in the Marketplace schema.yaml is now
    passed in as a $TAG environment variable during builds of the Marketplace
    deployer (rather than being fixed) so that when we build and test/verify
    with new versions of conjur, deployer, nginx, etc., we're actually testing
    and verifying with the desired versions that were just built, not the
    pinned, published versions that have been previously pushed to GCR.
  • The VERSION file is now set to 1.6.0, and the Conjur OSS version
    that is used by the Marketplace app is bumped to 1.6.0.
  • The Jenkinsfile jobs will now read the VERSION file to determine tags.
    For master branch, contents of VERSION are used directly. For any
    other branch, the contents of VERSION are used with a suffix of
    -<short-commit-ref>.
  • The version of Postgres used is changed from 9.4 to 10.1, since
    running with postgres:9.4 was failing. (Version 10.1 is the default
    version used in the conjur-oss helm charts).
  • For testing and development, the build.sh script now allows for the
    setting of the issuer common name used in the Conjur CA certificate.
    This can be set via a CERTIFICATE_COMMON_NAME environment variable.
    This will make it easier to configure DNS A records or entries in
    the local /etc/hosts file for testing access to Conjur.
  • README.md changes:
    • Corrects the manual helm install instructions so that the helm charts
      use the same image tags as used by the Marketplace application deployer.
    • Adds more details and corrects some inconsistencies in instructions
      for connecting to Conjur after helm installation.
  • CONTRIBUTING.md changes:
    • Adds some steps for configuring and connecting to Conjur
    • Removed support for flat dockerhub registries, since this won't work
      with the way that schema.yaml expects a hierarchichal GCR registry.
  • Marketplace application manifest:
    • Adds more details/clarifications on how to connect to Conjur
      and authenticate.

@diverdane diverdane requested a review from sgnn7 April 14, 2020 00:30
Copy link
Contributor

@sgnn7 sgnn7 left a comment

Choose a reason for hiding this comment

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

@diverdane Left a few comments

CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated

```shell
echo "$EXT_IP $COMMON_NAME" >> /etc/hosts
Copy link
Contributor

Choose a reason for hiding this comment

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

Ditto about sudo and noting that this could create duplicates and earlier entries will override newer ones. I think we may need a better CLI addition of a host.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed this to first look for an existing entry for the common name. If an entry exists, it is overwritten with a new external IP with sed. Otherwise, a new entry is added to /etc/hosts.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
conjur/templates/application.yaml Outdated Show resolved Hide resolved
@diverdane diverdane force-pushed the 41_pre_publish branch 13 times, most recently from a86c59b to 7d7b093 Compare April 17, 2020 21:42
@diverdane diverdane requested a review from sgnn7 April 17, 2020 21:52
CHANGELOG.md Show resolved Hide resolved
CHANGELOG.md Outdated
@@ -6,6 +6,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.

## [Unreleased]
### Changed
- The version of Conjur OSS used by the Marketplace app is bumped to 1.6.0.
- The CA certificate Common Name (CN) can now be manually configured as an environment variable for development testing via the build.sh script.
Copy link
Contributor

Choose a reason for hiding this comment

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

this line doesn't belong in the CHANGELOG, which should be used to describe user-facing changes to the project. is there a user impact of this change?

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 point. There is no user impact of this change. The difficulty I have sometimes is keeping a crisp distinction between users and contributors... in that I conflate contributors as "users of our open source code". :)

CHANGELOG.md Outdated Show resolved Hide resolved
Copy link
Contributor

@sgnn7 sgnn7 left a comment

Choose a reason for hiding this comment

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

@diverdane Getting really close!

CHANGELOG.md Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
apptest/deployer/schema.yaml Show resolved Hide resolved
Changes included:

* A `VERSION` text file is added to set the current published version
  for the Marketplace app.
* The publishedVersion field in the Marketplace schema.yaml is now
  passed in as a $TAG environment variable during builds of the Marketplace
  deployer (rather than being fixed) so that when we build and test/verify
  with new versions of conjur, deployer, nginx, etc., we're actually testing
  and verifying with the desired versions that were just built, not the
  pinned, published versions that have been previously pushed to GCR.
* The Marketplace app VERSION file is now set to `1.6.0`, and the Conjur
  OSS version that is used by the Marketplace app is bumped to `1.6`.
* The Jenkinsfile jobs will now read the VERSION file to determine tags.
  For master branch, contents of VERSION are used directly. For any
  other branch, the contents of VERSION are used with a suffix of
  `-<short-commit-ref>`.
* The version of Postgres used is changed from 9.4 to 10.1, since
  running with postgres:9.4 was failing. (Version 10.1 is the default
  version used in the conjur-oss helm charts).
* For testing and development, the build.sh script now allows for the
  setting of the issuer common name used in the Conjur CA certificate.
  This can be set via a CERTIFICATE_COMMON_NAME environment variable.
  This will make it easier to configure DNS A records or entries in
  the local /etc/hosts file for testing access to Conjur.
* README.md changes:
  - Corrects the manual helm install instructions so that the helm charts
    use the same image tags as used by the Marketplace application deployer.
  - Adds more details and corrects some inconsistencies in instructions
    for connecting to Conjur after helm installation.
* CONTRIBUTING.md changes:
  - Adds some steps for configuring and connecting to Conjur
  - Removed support for flat dockerhub registries, since this won't work
    with the way that schema.yaml expects a hierarchichal GCR registry.
* Marketplace application manifest:
  - Adds more details/clarifications on how to connect to Conjur
    and authenticate.
Copy link
Contributor

@sgnn7 sgnn7 left a comment

Choose a reason for hiding this comment

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

@diverdane LGTM! Hopefully this will be all that's needed.

@sgnn7
Copy link
Contributor

sgnn7 commented Apr 27, 2020

Edit: Looks like the build is failing though

@diverdane
Copy link
Contributor Author

Edit: Looks like the build is failing though

Re-ran, and it passed. I don't know why the scan for fixable vulnerabilities failed before, as the scan results looked identical between passing and failing cases.

@diverdane diverdane merged commit 279bf98 into master Apr 28, 2020
@diverdane diverdane deleted the 41_pre_publish branch April 28, 2020 12:22
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants