Skip to content

Added MD5 and SHA1 digested hashes inside header for HTTP HEAD reques… #1393

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

Merged
jdcasey merged 35 commits into from
Nov 4, 2019
Merged

Added MD5 and SHA1 digested hashes inside header for HTTP HEAD reques… #1393

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
8fcf358
Reuse the existing repo as implied one should add it into group
sswguo Oct 18, 2019
64966b2
NOS-2099:
gorgija Oct 18, 2019
c8ba4af
Test
gorgija Oct 18, 2019
f2cd6fe
Added MD5 and SHA1 digested hashes inside header for HTTP HEAD reques…
gorgija Oct 18, 2019
eb9b870
Filter the existing repos matching the conditions
sswguo Oct 21, 2019
5cd828f
Enhance the condition for comparing the repo
sswguo Oct 21, 2019
6400361
Fix Bad Request 400 error when uploading snapshot maven-metadata (#1389)
ruhan1 Oct 21, 2019
69dfcda
Allow disable of content index, and add prometheus metrics adapter
jdcasey Oct 21, 2019
989ea92
add managed dep on indy-subsys-metrics-prometheus
jdcasey Oct 21, 2019
bd527c9
Fix tests broken by default-disabled content index, and improve some …
jdcasey Oct 21, 2019
153be6d
Merge pull request #1394 from jdcasey/master
jdcasey Oct 21, 2019
23ccf23
Deprecated listContents in client module as it is not working now
ligangty Oct 22, 2019
b8a2925
Improve Prometheus metrics using node_prefix as a label to help with …
jdcasey Oct 22, 2019
85cd637
Tweak the condition and merge the keys of metadata "implied_by_stores"
sswguo Oct 23, 2019
913a43e
Merge pull request #1397 from jdcasey/master
jdcasey Oct 23, 2019
8b69574
Merge pull request #1390 from sswguo/implied_existing_repo
jdcasey Oct 23, 2019
a305c67
Merge branch 'NOS-2048' of https://github.com/geored/indy into NOS-2048
gorgija Oct 24, 2019
df7329a
Added Tests for checking http HEAD requests on '/api/browse/{packageT…
gorgija Oct 24, 2019
350fd29
Changes Requested
gorgija Oct 24, 2019
7086fd5
Changes Required
gorgija Oct 24, 2019
6011e4e
only do deploy when we're doing an image build, and add timestamps to…
jdcasey Oct 24, 2019
d799d49
Adding unit tests for version change in TrackedContentEntry
jdcasey Oct 24, 2019
e1e5ad2
Bug fix
gorgija Oct 25, 2019
dae3f04
InternalFeature isn't null value
gorgija Oct 25, 2019
2f62267
DefaultStoreValidatorTest
gorgija Oct 25, 2019
8345d1a
Fixing merge logic in TrackedContentEntry and fixing serialVersionUID…
jdcasey Oct 29, 2019
8664bf9
Merge pull request #1399 from jdcasey/master
jdcasey Oct 30, 2019
1849bc2
Merge pull request #1395 from ligangty/master
jdcasey Oct 30, 2019
cdd9f4c
Merge pull request #1392 from geored/NOS-2099
jdcasey Oct 31, 2019
a497b08
Merge branch 'NOS-2048' of https://github.com/geored/indy into NOS-2048
geored Nov 1, 2019
1589aa3
Added Digesting Algos for Repository contents inside 'ResponseHelper'…
geored Nov 1, 2019
f36d341
Added Digesting Algos for Repository contents inside 'ResponseHelper'…
geored Nov 1, 2019
97859d0
Path not directory
geored Nov 2, 2019
287b08a
Content Digest if path doesn't end with '/'
geored Nov 2, 2019
bd47010
Added headers in ContentAcessHandler
geored Nov 2, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 19 additions & 13 deletions Jenkinsfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,19 +22,6 @@ pipeline {
sh 'mvn -B -V verify -Prun-its -Pci'
}
}
stage('Deploy') {
when { branch 'master' }
steps {
echo "Deploy"
sh 'mvn help:effective-settings -B -V deploy -e'
}
}
stage('Archive') {
steps {
echo "Archive"
archiveArtifacts artifacts: "$artifact", fingerprint: true
}
}
stage('Check Image Build Hook') {
when {
expression { env.IMG_BUILD_HOOKS != null }
Expand All @@ -54,6 +41,25 @@ pipeline {
}
}
}
stage('Deploy') {
when {
allOf {
expression { img_build_hook != null }
expression { env.CHANGE_ID == null } // Not pull request
branch 'master'
}
}
steps {
echo "Deploy"
sh 'mvn help:effective-settings -B -V deploy -e'
}
}
stage('Archive') {
steps {
echo "Archive"
archiveArtifacts artifacts: "$artifact", fingerprint: true
}
}
stage('Build & Push Image') {
when {
allOf {
Expand Down
86 changes: 59 additions & 27 deletions ...rs/src/main/java/org/commonjava/indy/content/browse/bind/jaxrs/ContentBrowseResource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import org.commonjava.indy.bind.jaxrs.util.JaxRsRequestHelper;
import org.commonjava.indy.bind.jaxrs.util.REST;
import org.commonjava.indy.bind.jaxrs.util.ResponseHelper;
import org.commonjava.indy.content.ContentDigester;
import org.commonjava.indy.content.browse.ContentBrowseController;
import org.commonjava.indy.content.browse.model.ContentBrowseResult;
import org.commonjava.indy.model.core.PackageTypes;
Expand All @@ -39,6 +40,7 @@
import org.commonjava.indy.util.ApplicationHeader;
import org.commonjava.indy.util.UriFormatter;
import org.commonjava.maven.galley.event.EventMetadata;
import org.commonjava.maven.galley.io.checksum.ContentDigest;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand All @@ -58,7 +60,7 @@

@Api( value = "Indy Directory Content Browse", description = "Browse directory content in indy repository" )
@Path( "/api/browse/{packageType}/{type: (hosted|group|remote)}/{name}" )
@ApplicationScoped
//@ApplicationScoped
@REST
public class ContentBrowseResource
implements IndyResources
Expand All @@ -78,43 +80,67 @@ public class ContentBrowseResource
@Inject
private UriFormatter uriFormatter;


@Inject
protected JaxRsRequestHelper jaxRsRequestHelper;

@Inject
private ResponseHelper responseHelper;

@Inject
ContentDigester contentDigester;

@Context UriInfo uriInfo;
Copy link
Copy Markdown
Member

@jdcasey jdcasey Oct 21, 2019

Choose a reason for hiding this comment

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

Do we need to remove @ApplicationScoped from the class for this to work in a multi-threaded way?

Copy link
Copy Markdown
Member Author

@geored geored Oct 24, 2019

Choose a reason for hiding this comment

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

Right now its not problem multi-threaded that much as returning
"HTTP/1.1 500 Internal Server Error" on every each http HEAD request to that endpoint ("/api/browse/{packageType}/{type}/{name}/{path: (.*)}") , that's way i'm changing to one method which will handle both routes ( "/" and "/{path}/{path}/... ) with this setup.
For tests yes , i will provide them on next commit iteration for this PR

Copy link
Copy Markdown
Member

@jdcasey jdcasey Oct 24, 2019

Choose a reason for hiding this comment

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

I'm just wondering if we should inject these @context fields as method parameters to avoid cross-request pollution since the class is application scoped.

Copy link
Copy Markdown
Member

@jdcasey jdcasey Oct 24, 2019

Choose a reason for hiding this comment

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

As far as the tests, if you're still working on this can you mark it as WIP? When the tests come up clean it can be confusing whether it's ready to merge or not.

Copy link
Copy Markdown
Member Author

@geored geored Oct 24, 2019

Choose a reason for hiding this comment

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

"I'm just wondering if we should inject these @context fields as method parameters to avoid cross-request pollution since the class is application scoped."

  • it is one to one relation with every request, problem is ( what i noticed and i search but so far didn't find any specific reason to back up my claim ) when it is put as parameter reference ( im not sure is it @context or is it @httpservletrequest who is doing this ) inside @Head http request it is creating exception ( relating to "i cannot find relating info" this maybe is only specific for our environment ) and thats way i remove it rom @Head and place it on object ( which basically should be same ) but on other hand maybe you are right when saying that @applicationscoped is causing thread polution acros requests because we have one instance for each request....
    Yes , i will remove @ApplicationScoped and try without and let you know.

Copy link
Copy Markdown
Member Author

@geored geored Oct 24, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

"As far as the tests, if you're still working on this can you mark it as WIP? When the tests come up clean it can be confusing whether it's ready to merge or not."
John there is other NOS-1896 issue which is addresing same task but it is broather in sense that is asking for content checkup and deletion based on this MD5/SHA1 header checkup.
Sorry for not cleaned code , i have noticed latter on and i must do that because of hurry.

Copy link
Copy Markdown
Member Author

@geored geored Oct 24, 2019

Choose a reason for hiding this comment

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

Yes @jdcasey @Applicationscope is creating server problems with @context injected inside method as parameter , also second @Head method with @path("/") is generating server error even without @applicationscope. One other issue is that pathmapping ( @path("/{path (.*)} ) is returning :"415 Media Unsuported " response.


@Context HttpServletRequest request;

@ApiOperation( "Retrieve directory content under the given artifact store (type/name) and directory path." )
@ApiResponses( { @ApiResponse( code = 404, message = "Content is not available" ),
@ApiResponse( code = 200, response = String.class, message = "Rendered content listing" ) } )
@ApiResponses({@ApiResponse( code = 200, response = String.class, message = "Rendered content listing" ),@ApiResponse( code = 404, message = "Content is not available" )})
@HEAD
@Path( "/{path: (.*)}" )
@Path( "/{path (.*)}" )
public Response headForDirectory(
final @ApiParam( allowableValues = "maven,npm", required = true ) @PathParam( "packageType" )
String packageType,
final @ApiParam( allowableValues = "hosted,group,remote", required = true ) @PathParam( "type" )
String type, final @ApiParam( required = true ) @PathParam( "name" ) String name,
final @PathParam( "path" ) String path, @Context final UriInfo uriInfo,
@Context final HttpServletRequest request )
{
final @ApiParam( allowableValues = "maven,npm", required = true ) @PathParam( "packageType" ) String packageType,
final @ApiParam( allowableValues = "hosted,group,remote", required = true ) @PathParam( "type" ) String type,
final @ApiParam( required = true ) @PathParam( "name" ) String name,
final @PathParam( "path" ) String path
) throws IndyWorkflowException {
return processHead( packageType, type, name, path, uriInfo, request );
}

@ApiOperation( "Retrieve directory content under the given artifact store (type/name) and directory path." )
@ApiResponses( { @ApiResponse( code = 404, message = "Content is not available" ),
@ApiResponse( code = 200, response = String.class, message = "Rendered content listing" ) } )
@HEAD
@Path( "/" )
public Response headForRoot(
final @ApiParam( allowableValues = "maven,npm", required = true ) @PathParam( "packageType" )
String packageType,
final @ApiParam( allowableValues = "hosted,group,remote", required = true ) @PathParam( "type" )
String type, final @ApiParam( required = true ) @PathParam( "name" ) String name,
final @PathParam( "path" ) String path, @Context final UriInfo uriInfo,
@Context final HttpServletRequest request )
{
return processHead( packageType, type, name, "", uriInfo, request );
}



// @ApiOperation( "Retrieve directory content under the given artifact store (type/name) and directory path." )
// @ApiResponses( { @ApiResponse( code = 404, message = "Content is not available" ),
// @ApiResponse( code = 200, response = String.class, message = "Rendered content listing" ) } )
// @HEAD
// @Path( "/{path: (.*)}" )
// public Response headForDirectory(
// final @ApiParam( allowableValues = "maven,npm", required = true ) @PathParam( "packageType" )
// String packageType,
// final @ApiParam( allowableValues = "hosted,group,remote", required = true ) @PathParam( "type" )
// String type, final @ApiParam( required = true ) @PathParam( "name" ) String name,
// final @PathParam( "path" ) String path, final UriInfo uriInfo,
// @Context final HttpServletRequest request )
// {
// return processHead( packageType, type, name, path, uriInfo, request );
// }
//
// @ApiOperation( "Retrieve directory content under the given artifact store (type/name) and directory path." )
// @ApiResponses( { @ApiResponse( code = 404, message = "Content is not available" ),
// @ApiResponse( code = 200, response = String.class, message = "Rendered content listing" ) } )
// @HEAD
// @Path( "/" )
// public Response headForRoot(
// final @ApiParam( allowableValues = "maven,npm", required = true ) @PathParam( "packageType" )
// String packageType,
// final @ApiParam( allowableValues = "hosted,group,remote", required = true ) @PathParam( "type" )
// String type, final @ApiParam( required = true ) @PathParam( "name" ) String name,
// final @PathParam( "path" ) String path, @Context final UriInfo uriInfo,
// @Context final HttpServletRequest request )
// {
// return processHead( packageType, type, name, "", uriInfo, request );
// }

private Response processHead( final String packageType, final String type, final String name, final String path,
final UriInfo uriInfo, final HttpServletRequest request )
Expand All @@ -138,7 +164,13 @@ private Response processHead( final String packageType, final String type, final
.header( ApplicationHeader.content_length.key(),
Long.toString( content.length() ) )
.header( ApplicationHeader.last_modified.key(),
HttpUtils.formatDateHeader( new Date() ) );
HttpUtils.formatDateHeader( new Date() ) )
.header(ApplicationHeader.md5.key(),
contentDigester.digest(result.getStoreKey(), path, new EventMetadata()).getDigests().get(ContentDigest.MD5).toUpperCase() )
.header(ApplicationHeader.sha1.key(),
contentDigester.digest(result.getStoreKey(), path, new EventMetadata()).getDigests().get(ContentDigest.SHA_1).toUpperCase() )
;
;
response = builder.build();

}
Expand Down
4 changes: 3 additions & 1 deletion addons/content-index/src/main/conf/conf.d/content-index.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
[content-index]
#enabled=false

# This property is used to control if authoritative index is enabled. the authoritative index means:
# in terms of performance consideration, we will use content-index as the one-time check for the content
# access, and if not found in content indexing, will treat it as "no content" and bypass the following access to the storage.
#support.authoritative.indexes=true

# This property is used to enable content index warmer, which will scan all repos and load all artifacts
# into content index when startup.
# index.warmer.enable=true
# index.warmer.enabled=true
76 changes: 76 additions & 0 deletions ...ent-index/src/main/java/org/commonjava/indy/content/index/DefaultContentIndexManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

import org.commonjava.indy.action.IndyLifecycleException;
import org.commonjava.indy.action.ShutdownAction;
import org.commonjava.indy.content.index.conf.ContentIndexConfig;
import org.commonjava.indy.data.StoreDataManager;
import org.commonjava.indy.measure.annotation.Measure;
import org.commonjava.indy.model.core.ArtifactStore;
Expand Down Expand Up @@ -77,6 +78,9 @@ public class DefaultContentIndexManager
@Inject
private Instance<PackageIndexingStrategy> indexingStrategyComponents;

@Inject
private ContentIndexConfig config;

private Map<String, PackageIndexingStrategy> indexingStrategies;

private QueryFactory queryFactory;
Expand All @@ -98,6 +102,12 @@ public DefaultContentIndexManager( StoreDataManager storeDataManager, SpecialPat
@PostConstruct
public void constructed()
{
if ( !config.isEnabled() )
{
logger.debug( "Content indexing is disabled." );
return;
}

if ( indexingStrategyComponents != null )
{
Map<String, PackageIndexingStrategy> strats = new HashMap<>();
Expand Down Expand Up @@ -126,6 +136,12 @@ public String getId()
public void stop()
throws IndyLifecycleException
{
if ( !config.isEnabled() )
{
logger.debug( "Content indexing is disabled." );
return;
}

logger.debug( "Shutdown index cache" );
contentIndex.stop();
}
Expand All @@ -140,6 +156,12 @@ public int getShutdownPriority()
@Measure
public boolean removeIndexedStorePath( String rawPath, StoreKey key, Consumer<IndexedStorePath> pathConsumer )
{
if ( !config.isEnabled() )
{
logger.debug( "Content indexing is disabled." );
return false;
}

String path = getStrategyPath( key, rawPath );
IndexedStorePath topPath = new IndexedStorePath( key, path );
logger.trace( "Attempting to remove indexed path: {}", topPath );
Expand All @@ -158,6 +180,12 @@ public boolean removeIndexedStorePath( String rawPath, StoreKey key, Consumer<In

public String getStrategyPath( final StoreKey key, final String rawPath )
{
if ( !config.isEnabled() )
{
logger.debug( "Content indexing is disabled." );
return rawPath;
}

PackageIndexingStrategy strategy = indexingStrategies.get( key.getPackageType() );
if ( strategy == null )
{
Expand All @@ -174,6 +202,12 @@ public String getStrategyPath( final StoreKey key, final String rawPath )
@Measure
public void deIndexStorePath( final StoreKey key, final String rawPath )
{
if ( !config.isEnabled() )
{
logger.debug( "Content indexing is disabled." );
return;
}

String path = getStrategyPath( key, rawPath );
IndexedStorePath toRemove = new IndexedStorePath( key, path );
IndexedStorePath val = contentIndex.remove( toRemove );
Expand All @@ -184,6 +218,12 @@ public void deIndexStorePath( final StoreKey key, final String rawPath )
@Measure
public StoreKey getIndexedStoreKey( final StoreKey key, final String rawPath )
{
if ( !config.isEnabled() )
{
logger.debug( "Content indexing is disabled." );
return null;
}

String path = getStrategyPath( key, rawPath );
IndexedStorePath ispKey = new IndexedStorePath( key, path );
IndexedStorePath val = contentIndex.get( ispKey );
Expand All @@ -204,6 +244,12 @@ public StoreKey getIndexedStoreKey( final StoreKey key, final String rawPath )
@Measure
public void indexTransferIn( Transfer transfer, StoreKey...topKeys )
{
if ( !config.isEnabled() )
{
logger.debug( "Content indexing is disabled." );
return;
}

if ( transfer != null && transfer.exists() )
{
StoreKey key = LocationUtils.getKey( transfer );
Expand All @@ -219,6 +265,12 @@ public void indexTransferIn( Transfer transfer, StoreKey...topKeys )
@Measure
public void indexPathInStores( String rawPath, StoreKey originKey, StoreKey... topKeys )
{
if ( !config.isEnabled() )
{
logger.debug( "Content indexing is disabled." );
return;
}

String path = getStrategyPath( originKey, rawPath );

IndexedStorePath origin = new IndexedStorePath( originKey, path );
Expand All @@ -237,6 +289,12 @@ public void indexPathInStores( String rawPath, StoreKey originKey, StoreKey... t
@Measure
public void clearAllIndexedPathInStore( ArtifactStore store )
{
if ( !config.isEnabled() )
{
logger.debug( "Content indexing is disabled." );
return;
}

StoreKey sk = store.getKey();

long total = iterateRemove( () -> queryFactory.from( IndexedStorePath.class )
Expand All @@ -259,6 +317,12 @@ public void clearAllIndexedPathInStore( ArtifactStore store )
@Measure
public void clearAllIndexedPathWithOriginalStore( ArtifactStore originalStore )
{
if ( !config.isEnabled() )
{
logger.debug( "Content indexing is disabled." );
return;
}

StoreKey osk = originalStore.getKey();

long total = iterateRemove( () -> queryFactory.from( IndexedStorePath.class )
Expand Down Expand Up @@ -303,6 +367,12 @@ private long iterateRemove( final Supplier<Query> queryFunction )
@Measure
public void clearAllIndexedPathInStoreWithOriginal( ArtifactStore store, ArtifactStore originalStore )
{
if ( !config.isEnabled() )
{
logger.debug( "Content indexing is disabled." );
return;
}

StoreKey sk = store.getKey();
StoreKey osk = originalStore.getKey();

Expand Down Expand Up @@ -337,6 +407,12 @@ public void clearAllIndexedPathInStoreWithOriginal( ArtifactStore store, Artifac
@Measure
public void clearIndexedPathFrom( String rawPath, Set<Group> groups, Consumer<IndexedStorePath> pathConsumer )
{
if ( !config.isEnabled() )
{
logger.debug( "Content indexing is disabled." );
return;
}

if ( groups == null || groups.isEmpty() )
{
return;
Expand Down
Loading