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 Dyadic EKM buildpack #365

Closed
wants to merge 4 commits into from
Closed

Add Dyadic EKM buildpack #365

wants to merge 4 commits into from

Conversation

peer85
Copy link
Contributor

@peer85 peer85 commented Dec 18, 2016

No description provided.

@cfdreddbot
Copy link

Hey peer85!

Thanks for submitting this pull request!

All pull request submitters and commit authors must have a Contributor License Agreement (CLA) on-file with us. Please sign the appropriate CLA (individual or corporate).

When sending signed CLA please provide your github username in case of individual CLA or the list of github usernames that can make pull requests on behalf of your organization.

If you are confident that you're covered under a Corporate CLA, please make sure you've publicized your membership in the appropriate Github Org, per these instructions.

Once you've publicized your membership, one of the owners of this repository can close and reopen this pull request, and dreddbot will take another look.

Copy link
Member

@nebhale nebhale left a comment

Choose a reason for hiding this comment

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

In addition to the items mentioned below (which are all cosmetic really), there are no tests. In the interests of getting this merged as quickly as possible I'll make the changes and write the tests (the Luna support is a good template, so it should go quickly). I'd encourage you to review the updates once they are complete so that future PR's around this code go in smoothly.

---
version: 1.+
repository_root: https://repo.dyadicsec.com/cust/pcf
logging_enabled: false
Copy link
Member

Choose a reason for hiding this comment

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

I don't believe that you've added support for configuring logging, so this entry can be removed.

download_tar
setup_ext_dir

@droplet.copy_resources
Copy link
Member

Choose a reason for hiding this comment

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

This file generally needs a reformat. If you're using IntelliJ, we've checked the formatting rules into source control, so you can use the code formatter there to fix it all to our standard.

end

def write_conf(servers,send_timeout,recv_timeout,retries)
conf_file.open(File::CREAT | File::WRONLY) do |f|
Copy link
Member

Choose a reason for hiding this comment

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

Need to ensure that the parent directory exists before writing with FileUtils.mkdir_p conf_file.parent.

end

def write_cert(cert)
cert_file.open(File::CREAT | File::WRONLY) do |f|
Copy link
Member

Choose a reason for hiding this comment

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

Need to ensure that the parent directory exists before writing with FileUtils.mkdir_p cert_file.parent.

end

def write_key(key)
key_file.open(File::CREAT | File::WRONLY) do |f|
Copy link
Member

Choose a reason for hiding this comment

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

Need to ensure that the parent directory exists before writing with FileUtils.mkdir_p key_file.parent.

# (see JavaBuildpack::Component::VersionedDependencyComponent#supports?)
def supports?
@application.services.one_service? FILTER
#true
Copy link
Member

Choose a reason for hiding this comment

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

Remove commented out code.

# (see JavaBuildpack::Component::BaseComponent#release)
def release
@droplet
.java_opts
Copy link
Member

Choose a reason for hiding this comment

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

All of the .java_opts can be combined into a single chained invocation.


def write_conf(servers,send_timeout,recv_timeout,retries)
conf_file.open(File::CREAT | File::WRONLY) do |f|
f.write "servers = " + servers + "\n"
Copy link
Member

Choose a reason for hiding this comment

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

This multiline string can be rewritten as a single Ruby multi-line interpolation like:

          f.write <<EOS
servers      = #{servers}
send_timeout = #{send_timeout}
recv_timeout = #{recv_timeout}
retries      = #{retries}
EOS

nebhale pushed a commit that referenced this pull request Dec 20, 2016
This change adds a framework that detects service instances for the Dyadic EKM
and configures an application to use it as a Java Security Provider.

[#365]
@nebhale nebhale closed this in 922b96b Dec 20, 2016
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.

3 participants